Re: [Libguestfs] [PATCH v3 5/5] appliance: Added filesystem_walk command tests

2016-04-05 Thread noxdafox

On 05/04/16 20:33, Pino Toscano wrote:

On Tuesday 05 April 2016 18:47:32 Matteo Cafasso wrote:

The tests check that the filesystem_walk command is able to retrieve
information regarding both existing and deleted files.

A NTFS image is used as Ext3+ filesystems deletion is more aggressive
in terms of metadata removal.

Signed-off-by: Matteo Cafasso 
---
  tests/tsk/Makefile.am |  3 +-
  tests/tsk/test-filesystem-walk.sh | 62 +++
  2 files changed, 64 insertions(+), 1 deletion(-)
  create mode 100755 tests/tsk/test-filesystem-walk.sh

diff --git a/tests/tsk/Makefile.am b/tests/tsk/Makefile.am
index 0cd7c03..f9b2fef 100644
--- a/tests/tsk/Makefile.am
+++ b/tests/tsk/Makefile.am
@@ -18,7 +18,8 @@
  include $(top_srcdir)/subdir-rules.mk

  TESTS = \
-   test-download-inode.sh
+   test-download-inode.sh \
+   test-filesystem-walk.sh

  TESTS_ENVIRONMENT = $(top_builddir)/run --test

diff --git a/tests/tsk/test-filesystem-walk.sh 
b/tests/tsk/test-filesystem-walk.sh
new file mode 100755
index 000..ab7c1a9
--- /dev/null
+++ b/tests/tsk/test-filesystem-walk.sh
@@ -0,0 +1,62 @@
+#!/bin/bash -
+# libguestfs
+# Copyright (C) 2016 Red Hat Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+# Test the filesystem-walk command.
+
+if [ -n "$SKIP_TEST_FILESYSTEM_WALK_SH" ]; then
+echo "$0: test skipped because environment variable is set."
+exit 77
+fi
+
+# Skip if TSK is not supported by the appliance.
+if ! guestfish add /dev/null : run : available "libtsk"; then
+echo "$0: skipped because TSK is not available in the appliance"
+exit 77
+fi
+
+if [ ! -s ../../test-data/phony-guests/windows.img ]; then
+echo "$0: skipped because windows.img is zero-sized"
+exit 77
+fi
+
+# create and delete a file then list the filesystem content
+output=$(guestfish --ro -a ../../test-data/phony-guests/windows.img \
+   run :\
+   mount /dev/sda2 / :  \
+   write /test.txt "foobar" :   \
+   rm /test.txt :   \
+   umount / :   \
+   filesystem-walk /dev/sda2)

This is a bit unreadable, a better approach is to read commands from
stdin; see for example fish/test-copy.sh.


+
+# test $MFT is in the list
+echo $output | grep -q "{ tsk_inode: 0 tsk_type: r tsk_size: .* tsk_name: \$MFT 
tsk_allocated: 1 }"

Hmm are you sure this works when tracing is disabled?  The default
output in guestfish for structs is each field in a single line.


I tried it without tracing and it works as well. Just to be sure, I'll 
add the -z flag to grep to ignore newlines.




Unless you compare the whole output like other tests do, a better
solution could be write this test using a scripting language like Perl:
I think most, if not all, of the non-bash tests are Perl-based, and it
would allow to do better checks for the return values.
I'm not familiar with Perl therefore it would require a bit more of time 
for little value.
I could do it in Python but I believe bash for such simple cases is way 
more readable and effective.


Thanks,


___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs


___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH 6/7] v2v: quiet virtio net and balloon devices wizards

2016-04-05 Thread Roman Kagan
On Tue, Apr 05, 2016 at 07:53:16PM +0300, Roman Kagan wrote:
> On Tue, Apr 05, 2016 at 05:33:28PM +0200, Cedric Bosdonnat wrote:
> > On Tue, 2016-04-05 at 17:19 +0300, Roman Kagan wrote:
> > > On Tue, Apr 05, 2016 at 01:47:32PM +0200, Cédric Bosdonnat wrote:
> > > > Setting the ConfigFlags to 0x40 for those will make windows quiet
> > > > at the first boot about those new devices. The wizard must not be
> > > > presented to the user since the needed drivers will automatically
> > > > be installed at firstboot... or worse, the wizard can even block
> > > > the installer.
> > > 
> > > What installer?
> > 
> > At least the VMDP installer running at firstboot is blocked by these
> > wizards.
> 
> I see.  Perhaps this needs to be fixed in that installer, then (no, I
> have no idea how or whether it's possible at all)?
> 
> > > You're trying circumvent the usual PnP process people are used to. 
> > >  I'm
> > > not sure it's worth adding yet more unreliable hacks (yes, basically
> > > the
> > > whole v2v/windows_virtio.ml is a hack).
> > 
> > Setting those keys forces windows to ignore the virtio net and balloon
> > devices for the first boot time. Running the VMDP installer (or the RH
> > equivalent one) will install the needed drivers and all will be fine
> > after that.
> 
> I'll talk to our guys doing similar things.  I wonder what they do --
> they must have similar problems.

Indeed they do.  The solution we're considering ATM is

1) make the installer support running on a system where the drivers are
   already installed

2) make sure firstboot scripts aren't run before PnP is complete

This should resolve the conflict you wrote about, by making the drivers
brought in by v2v fully installed via standard Windows PnP manager
first, and running the installer afterwards.

Item (2) is also useful to avoid similar conflicts with other firstboot
scripts, because PnP often requires a reboot, and those firstboot
scripts may not tolerate a reboot in the middle of execution.

Will this work for you?

Roman.

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs


Re: [Libguestfs] [PATCH v3 1/5] generator: Added tsk_dirent struct

2016-04-05 Thread noxdafox

On 05/04/16 19:33, Pino Toscano wrote:

On Tuesday 05 April 2016 18:47:28 Matteo Cafasso wrote:

The tsk_dirent struct contains the information gathered via TSK APIs.

The struct contains the following fields:
  * tsk_inode: inode of a file
  * tsk_type: type of file such as for dirwalk command
  * tsk_size: file size in bytes
  * tsk_name: path relative to its disk partition
  * tsk_allocated: whether the file has been deleted

Signed-off-by: Matteo Cafasso 
---
  generator/structs.ml | 16 ++--
  1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/generator/structs.ml b/generator/structs.ml
index 6017ba6..d986fd9 100644
--- a/generator/structs.ml
+++ b/generator/structs.ml
@@ -442,8 +442,20 @@ let structs = [
  "im_device", FString;
  "im_volume", FString;
  ];
-s_camel_name = "InternalMountable";
-  };
+s_camel_name = "InternalMountable" };

Unneeded change.


+  (* The Sleuth Kit directory entry information. *)
+  { defaults with
+s_name = "tsk_dirent";
+s_cols = [
+"tsk_inode", FUInt64;
+"tsk_type", FChar;
+"tsk_size", FInt64;
+"tsk_name", FString;
+"tsk_allocated", FUInt32;

Once added to the public API, a struct cannot be extended anymore
(it would break the ABI).  IMHO it would make more sense to have
a tsk_flags instead of tsk_allocated, documenting the values of the
flags/bits set: this way, adding a new simple boolean flag won't
require a new tsk_dirent2 (see e.g. the application2 struct).


The application2 structs shows pretty well the issue but does not have 
any bitwise flag.
Is there an example I could use as a reference? Especially I'd like to 
see how flags are exposed and documented.




Thanks,


___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs


___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH v3 5/5] appliance: Added filesystem_walk command tests

2016-04-05 Thread Pino Toscano
On Tuesday 05 April 2016 18:47:32 Matteo Cafasso wrote:
> The tests check that the filesystem_walk command is able to retrieve
> information regarding both existing and deleted files.
> 
> A NTFS image is used as Ext3+ filesystems deletion is more aggressive
> in terms of metadata removal.
> 
> Signed-off-by: Matteo Cafasso 
> ---
>  tests/tsk/Makefile.am |  3 +-
>  tests/tsk/test-filesystem-walk.sh | 62 
> +++
>  2 files changed, 64 insertions(+), 1 deletion(-)
>  create mode 100755 tests/tsk/test-filesystem-walk.sh
> 
> diff --git a/tests/tsk/Makefile.am b/tests/tsk/Makefile.am
> index 0cd7c03..f9b2fef 100644
> --- a/tests/tsk/Makefile.am
> +++ b/tests/tsk/Makefile.am
> @@ -18,7 +18,8 @@
>  include $(top_srcdir)/subdir-rules.mk
> 
>  TESTS = \
> - test-download-inode.sh
> + test-download-inode.sh \
> + test-filesystem-walk.sh
> 
>  TESTS_ENVIRONMENT = $(top_builddir)/run --test
> 
> diff --git a/tests/tsk/test-filesystem-walk.sh 
> b/tests/tsk/test-filesystem-walk.sh
> new file mode 100755
> index 000..ab7c1a9
> --- /dev/null
> +++ b/tests/tsk/test-filesystem-walk.sh
> @@ -0,0 +1,62 @@
> +#!/bin/bash -
> +# libguestfs
> +# Copyright (C) 2016 Red Hat Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 
> USA.
> +
> +# Test the filesystem-walk command.
> +
> +if [ -n "$SKIP_TEST_FILESYSTEM_WALK_SH" ]; then
> +echo "$0: test skipped because environment variable is set."
> +exit 77
> +fi
> +
> +# Skip if TSK is not supported by the appliance.
> +if ! guestfish add /dev/null : run : available "libtsk"; then
> +echo "$0: skipped because TSK is not available in the appliance"
> +exit 77
> +fi
> +
> +if [ ! -s ../../test-data/phony-guests/windows.img ]; then
> +echo "$0: skipped because windows.img is zero-sized"
> +exit 77
> +fi
> +
> +# create and delete a file then list the filesystem content
> +output=$(guestfish --ro -a ../../test-data/phony-guests/windows.img \
> +   run :\
> +   mount /dev/sda2 / :  \
> +   write /test.txt "foobar" :   \
> +   rm /test.txt :   \
> +   umount / :   \
> +   filesystem-walk /dev/sda2)

This is a bit unreadable, a better approach is to read commands from
stdin; see for example fish/test-copy.sh.

> +
> +# test $MFT is in the list
> +echo $output | grep -q "{ tsk_inode: 0 tsk_type: r tsk_size: .* tsk_name: 
> \$MFT tsk_allocated: 1 }"

Hmm are you sure this works when tracing is disabled?  The default
output in guestfish for structs is each field in a single line.

Unless you compare the whole output like other tests do, a better
solution could be write this test using a scripting language like Perl:
I think most, if not all, of the non-bash tests are Perl-based, and it
would allow to do better checks for the return values.

Thanks,
-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH v3 4/5] appliance: Added filesystem_walk command

2016-04-05 Thread Pino Toscano
"appliance: Added filesystem_walk command"

-> "New API: filesystem_walk"

On Tuesday 05 April 2016 18:47:31 Matteo Cafasso wrote:
> The filesystem_walk command is the appliance's
> counterpart of the daemon's internal_filesystem_walk command.

It is not the counterpart in the appliance, but in the library.
The appliance is the small VM used to do all the operations with the
disk images, and the guestfsd daemon runs there.

This non-daemon API exists only in the library, hence in userspace.

> It writes the daemon's command output on a temporary file and parses it,
> deserialising the XDR formatted tsk_dirent structs.
> 
> It returns to the caller the list of tsk_dirent structs generated by the
> internal_filesystem_walk command.
> 
> Signed-off-by: Matteo Cafasso 
> ---
>  generator/actions.ml |  69 +
>  src/Makefile.am  |   1 +
>  src/tsk.c| 123 
> +++
>  3 files changed, 193 insertions(+)
>  create mode 100644 src/tsk.c
> 
> diff --git a/generator/actions.ml b/generator/actions.ml
> index 8073370..afff402 100644
> --- a/generator/actions.ml
> +++ b/generator/actions.ml
> @@ -3546,6 +3546,75 @@ The environment variable C controls 
> the default
>  value: If C is set, then that is the default.
>  Else F is the default." };
> 
> +  { defaults with
> +name = "filesystem_walk"; added = (1, 33, 18);
> +style = RStructList ("dirents", "tsk_dirent"), [Mountable "device";], [];
> +optional = Some "libtsk";
> +progress = true; cancellable = true;
> +shortdesc = "walk through the filesystem content";
> +longdesc = "\
> +Walk through the internal structures of a disk partition
> +(eg. F) in order to return a list of all the files
> +and directories stored within.
> +
> +It is not necessary to mount the disk partition to run this command.
> +
> +All entries in the filesystem are returned, excluding C<.> and
> +C<..>. This function can list deleted or unaccessible files.
> +The entries are I sorted.
> +
> +If the entry is not allocated (ex: it has been deleted),
> +its inode, type or size might not be recovered correctly.
> +In such case, the inode might be 0, the size might be -1 and the type
> +might be unidentified 'u'.

Use POD C<..> for the special values 0, -1, 'u'.

> +This call returns as well basic file type information about each
> +file.  The C field will contain one of the following characters:
> +
> +=over 4
> +
> +=item 'b'
> +
> +Block special
> +
> +=item 'c'
> +
> +Char special
> +
> +=item 'd'
> +
> +Directory
> +
> +=item 'f'
> +
> +FIFO (named pipe)
> +
> +=item 'l'
> +
> +Symbolic link
> +
> +=item 'r'
> +
> +Regular file
> +
> +=item 's'
> +
> +Socket
> +
> +=item 'h'
> +
> +Shadow inode (Solaris)
> +
> +=item 'w'
> +
> +Whiteout inode (BSD)
> +
> +=item 'u'
> +
> +Unknown file type
> +
> +=back" };
> +
>  ]
> 
>  (* daemon_functions are any functions which cause some action
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 3b4cd10..9f8af4c 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -130,6 +130,7 @@ libguestfs_la_SOURCES = \
>   structs-copy.c \
>   structs-free.c \
>   tmpdirs.c \
> + tsk.c \
>   whole-file.c \
>   libguestfs.syms
> 
> diff --git a/src/tsk.c b/src/tsk.c
> new file mode 100644
> index 000..4d7fd7c
> --- /dev/null
> +++ b/src/tsk.c
> @@ -0,0 +1,123 @@
> +/* libguestfs
> + * Copyright (C) 2016 Red Hat Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 
> USA
> + */
> +
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "full-read.h"
> +
> +#include "guestfs.h"
> +#include "guestfs_protocol.h"
> +#include "guestfs-internal.h"
> +#include "guestfs-internal-all.h"
> +#include "guestfs-internal-actions.h"
> +
> +static struct guestfs_tsk_dirent_list *parse_filesystem_walk (guestfs_h *, 
> char *, size_t );

Extra space at the end.

> +int deserialise_dirent_list (guestfs_h *, char *, size_t , struct 
> guestfs_tsk_dirent_list *);

deserialise_dirent_list can be static.

> +
> +struct guestfs_tsk_dirent_list *
> +guestfs_impl_filesystem_walk (guestfs_h *g, const char *mountable)
> +{
> 

Re: [Libguestfs] [PATCH 6/7] v2v: quiet virtio net and balloon devices wizards

2016-04-05 Thread Roman Kagan
On Tue, Apr 05, 2016 at 05:33:28PM +0200, Cedric Bosdonnat wrote:
> On Tue, 2016-04-05 at 17:19 +0300, Roman Kagan wrote:
> > On Tue, Apr 05, 2016 at 01:47:32PM +0200, Cédric Bosdonnat wrote:
> > > Setting the ConfigFlags to 0x40 for those will make windows quiet
> > > at the first boot about those new devices. The wizard must not be
> > > presented to the user since the needed drivers will automatically
> > > be installed at firstboot... or worse, the wizard can even block
> > > the installer.
> > 
> > What installer?
> 
> At least the VMDP installer running at firstboot is blocked by these
> wizards.

I see.  Perhaps this needs to be fixed in that installer, then (no, I
have no idea how or whether it's possible at all)?

> > You're trying circumvent the usual PnP process people are used to. 
> >  I'm
> > not sure it's worth adding yet more unreliable hacks (yes, basically
> > the
> > whole v2v/windows_virtio.ml is a hack).
> 
> Setting those keys forces windows to ignore the virtio net and balloon
> devices for the first boot time. Running the VMDP installer (or the RH
> equivalent one) will install the needed drivers and all will be fine
> after that.

I'll talk to our guys doing similar things.  I wonder what they do --
they must have similar problems.

> > > @@ -196,6 +196,14 @@ and add_viostor_to_critical_device_database g
> > > root current_cs major =
> > >[ "0", REG_SZ (sprintf
> > > "PCI\\VEN_1AF4_1001_00021AF4_00\\%s&20" subkey);
> > >  "Count", REG_DWORD 0x1_l;
> > >  "NextInstance", REG_DWORD 0x1_l ];
> > > +
> > > +  [ current_cs; "Enum"; "PCI";
> > > "VEN_1AF4_1000_00011AF4_00"; subkey ^ "&18" ],
> > > +  [ "ConfigFlags", REG_DWORD 0x40_l ];
> > > +  [ current_cs; "Enum"; "PCI";
> > > "VEN_1AF4_1001_00021AF4_00"; subkey ^ "&20" ],
> > > +  [ "ConfigFlags", REG_DWORD 0x0_l;
> > > +"Service", REG_SZ driver_name ];
> > > +  [ current_cs; "Enum"; "PCI";
> > > "VEN_1AF4_1002_00051AF4_00"; subkey ^ "&28" ],
> > > +  [ "ConfigFlags", REG_DWORD 0x40_l ];
> > 
> > I'm curious how reliable those keys are; what are the chances that
> > the
> > devices get assigned different instance ids?  I couldn't find any
> > sources indicating that those instance ids are assigned in any
> > predictable manner.
> 
> I have no idea how they are computed, but they are common accross all
> windows versions except for the subkey that changes with windows
> versions.

OK I just added another network adapter, and it got &38 at the end.  So
this doesn't look like a reliable procedure, and you need to figure out
a better approach.

Roman.

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs


Re: [Libguestfs] [PATCH v3 3/5] daemon: Added internal_filesystem_walk command

2016-04-05 Thread Pino Toscano
"daemon: Added internal_filesystem_walk command"

-> "New API: internal_filesystem_walk"

On Tuesday 05 April 2016 18:47:30 Matteo Cafasso wrote:
> The internal_filesystem_walk command walks through the FS structures
> of a disk partition and returns all the files or directories
> which could be found.
> 
> The command is able to retrieve information regarding deleted
> or unaccessible files where other commands such as stat or find
> would fail.
> 
> The gathered list of tsk_dirent structs is serialised into XDR format
> and written to a file by the appliance.
> 
> Signed-off-by: Matteo Cafasso 
> ---
>  daemon/Makefile.am   |   4 +-
>  daemon/tsk.c | 233 
> +++
>  generator/actions.ml |   9 ++
>  src/MAX_PROC_NR  |   2 +-
>  4 files changed, 246 insertions(+), 2 deletions(-)
>  create mode 100644 daemon/tsk.c
> 
> diff --git a/daemon/Makefile.am b/daemon/Makefile.am
> index beb7962..03bf71f 100644
> --- a/daemon/Makefile.am
> +++ b/daemon/Makefile.am
> @@ -179,6 +179,7 @@ guestfsd_SOURCES = \
>   sync.c \
>   syslinux.c \
>   tar.c \
> + tsk.c \
>   truncate.c \
>   umask.c \
>   upload.c \
> @@ -209,7 +210,8 @@ guestfsd_LDADD = \
>   $(LIB_CLOCK_GETTIME) \
>   $(LIBINTL) \
>   $(SERVENT_LIB) \
> - $(PCRE_LIBS)
> + $(PCRE_LIBS) \
> + $(TSK_LIBS)
> 
>  guestfsd_CPPFLAGS = \
>   -I$(top_srcdir)/gnulib/lib \
> diff --git a/daemon/tsk.c b/daemon/tsk.c
> new file mode 100644
> index 000..cd4879b
> --- /dev/null
> +++ b/daemon/tsk.c
> @@ -0,0 +1,233 @@
> +/* libguestfs - the guestfsd daemon
> + * Copyright (C) 2016 Red Hat Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 
> USA.
> + */
> +
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "guestfs_protocol.h"
> +#include "daemon.h"
> +#include "actions.h"
> +#include "optgroups.h"
> +
> +#ifdef HAVE_LIBTSK
> +
> +#include 
> +
> +static int open_filesystem (const char *, TSK_IMG_INFO **, TSK_FS_INFO **);
> +static TSK_WALK_RET_ENUM fswalk_callback (TSK_FS_FILE *, const char *, void 
> *);
> +static char file_type (TSK_FS_FILE *);
> +static int send_dirent_info (guestfs_int_tsk_dirent *);
> +static void reply_with_tsk_error (const char *);
> +
> +int
> +do_internal_filesystem_walk (const mountable_t *mountable)
> +{
> +  int ret = -1;
> +  TSK_FS_INFO *fs = NULL;
> +  TSK_IMG_INFO *img = NULL;  /* Used internally by tsk_fs_dir_walk */
> +  int flags = TSK_FS_DIR_WALK_FLAG_ALLOC | TSK_FS_DIR_WALK_FLAG_UNALLOC |
> +TSK_FS_DIR_WALK_FLAG_RECURSE | TSK_FS_DIR_WALK_FLAG_NOORPHAN;
> +
> +  ret = open_filesystem (mountable->device, , );
> +  if (ret < 0)
> +return ret;
> +
> +  reply (NULL, NULL);  /* Reply message. */
> +
> +  ret = tsk_fs_dir_walk (fs, fs->root_inum, flags, fswalk_callback, NULL);
> +  if (ret == 0)
> +ret = send_file_end (0);  /* File transfer end. */
> +  else
> +send_file_end (1);  /* Cancel file transfer. */
> +
> +  fs->close (fs);
> +  img->close (img);
> +
> +  return ret;
> +}
> +
> +/* Inspect the device and initialises the img and fs structures.
> + * Return 0 on success, -1 on error.
> + */
> +static int
> +open_filesystem (const char *device, TSK_IMG_INFO **img, TSK_FS_INFO **fs)
> +{
> +  const char *images[] = { device };
> +
> +  *img = tsk_img_open (1, images, TSK_IMG_TYPE_DETECT , 0);

Extra space here ...^

> +  if (*img == NULL) {
> +reply_with_tsk_error ("tsk_image_open");
> +return -1;
> +  }
> +
> +  *fs = tsk_fs_open_img (*img, 0, TSK_FS_TYPE_DETECT);
> +  if (*fs == NULL) {
> +reply_with_tsk_error ("tsk_fs_open_img");
> +(*img)->close (*img);
> +return -1;
> +  }
> +
> +  return 0;
> +}
> +
> +/* Filesystem walk callback, it gets called on every FS node.
> + * Parse the node, encode it into an XDR structure and send it to the 
> appliance.
> + * Return TSK_WALK_CONT on success, TSK_WALK_ERROR on error.
> + */
> +static TSK_WALK_RET_ENUM
> +fswalk_callback (TSK_FS_FILE *fsfile, const char *path, void *data)
> +{
> +  int ret = 0;
> +  CLEANUP_FREE char *fname = NULL;
> +  struct guestfs_int_tsk_dirent dirent;
> +
> +  /* Ignore ./ and ../ */
> +  

Re: [Libguestfs] [PATCH v3 2/5] configure: Added libtsk compile-time check

2016-04-05 Thread Pino Toscano
On Tuesday 05 April 2016 18:47:29 Matteo Cafasso wrote:
> Ensure libtsk is available at compile time.
> If not, daemon routines depending on it won't be available.
> 
> Signed-off-by: Matteo Cafasso 
> ---

Please also add the optional dependency to the documentation, see
docs/guestfs-building.pod.

>  m4/guestfs_daemon.m4 | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/m4/guestfs_daemon.m4 b/m4/guestfs_daemon.m4
> index 88936b2..09cfecd 100644
> --- a/m4/guestfs_daemon.m4
> +++ b/m4/guestfs_daemon.m4
> @@ -118,3 +118,11 @@ PKG_CHECK_MODULES([SD_JOURNAL], [libsystemd],[
>  AC_MSG_WARN([systemd journal library not found, some features will 
> be disabled])
>  ])
>  ])
> +
> +dnl libtsk sleuthkit library (optional)
> +AC_CHECK_LIB([tsk],[tsk_version_print],[
> +AC_CHECK_HEADER([tsk/libtsk.h],[
> +AC_SUBST([TSK_LIBS], [-ltsk])
> +AC_DEFINE([HAVE_LIBTSK], [1], [Define to 1 if The Sleuth Kit  
> library (libtsk) is available.])

Extra space here ...^

Thanks,
-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH v3 1/5] generator: Added tsk_dirent struct

2016-04-05 Thread Pino Toscano
On Tuesday 05 April 2016 18:47:28 Matteo Cafasso wrote:
> The tsk_dirent struct contains the information gathered via TSK APIs.
> 
> The struct contains the following fields:
>  * tsk_inode: inode of a file
>  * tsk_type: type of file such as for dirwalk command
>  * tsk_size: file size in bytes
>  * tsk_name: path relative to its disk partition
>  * tsk_allocated: whether the file has been deleted
> 
> Signed-off-by: Matteo Cafasso 
> ---
>  generator/structs.ml | 16 ++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/generator/structs.ml b/generator/structs.ml
> index 6017ba6..d986fd9 100644
> --- a/generator/structs.ml
> +++ b/generator/structs.ml
> @@ -442,8 +442,20 @@ let structs = [
>  "im_device", FString;
>  "im_volume", FString;
>  ];
> -s_camel_name = "InternalMountable";
> -  };
> +s_camel_name = "InternalMountable" };

Unneeded change.

> +  (* The Sleuth Kit directory entry information. *)
> +  { defaults with
> +s_name = "tsk_dirent";
> +s_cols = [
> +"tsk_inode", FUInt64;
> +"tsk_type", FChar;
> +"tsk_size", FInt64;
> +"tsk_name", FString;
> +"tsk_allocated", FUInt32;

Once added to the public API, a struct cannot be extended anymore
(it would break the ABI).  IMHO it would make more sense to have
a tsk_flags instead of tsk_allocated, documenting the values of the
flags/bits set: this way, adding a new simple boolean flag won't
require a new tsk_dirent2 (see e.g. the application2 struct).

Thanks,
-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] [PATCH v3 4/5] appliance: Added filesystem_walk command

2016-04-05 Thread Matteo Cafasso
The filesystem_walk command is the appliance's
counterpart of the daemon's internal_filesystem_walk command.

It writes the daemon's command output on a temporary file and parses it,
deserialising the XDR formatted tsk_dirent structs.

It returns to the caller the list of tsk_dirent structs generated by the
internal_filesystem_walk command.

Signed-off-by: Matteo Cafasso 
---
 generator/actions.ml |  69 +
 src/Makefile.am  |   1 +
 src/tsk.c| 123 +++
 3 files changed, 193 insertions(+)
 create mode 100644 src/tsk.c

diff --git a/generator/actions.ml b/generator/actions.ml
index 8073370..afff402 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -3546,6 +3546,75 @@ The environment variable C controls the 
default
 value: If C is set, then that is the default.
 Else F is the default." };

+  { defaults with
+name = "filesystem_walk"; added = (1, 33, 18);
+style = RStructList ("dirents", "tsk_dirent"), [Mountable "device";], [];
+optional = Some "libtsk";
+progress = true; cancellable = true;
+shortdesc = "walk through the filesystem content";
+longdesc = "\
+Walk through the internal structures of a disk partition
+(eg. F) in order to return a list of all the files
+and directories stored within.
+
+It is not necessary to mount the disk partition to run this command.
+
+All entries in the filesystem are returned, excluding C<.> and
+C<..>. This function can list deleted or unaccessible files.
+The entries are I sorted.
+
+If the entry is not allocated (ex: it has been deleted),
+its inode, type or size might not be recovered correctly.
+In such case, the inode might be 0, the size might be -1 and the type
+might be unidentified 'u'.
+
+This call returns as well basic file type information about each
+file.  The C field will contain one of the following characters:
+
+=over 4
+
+=item 'b'
+
+Block special
+
+=item 'c'
+
+Char special
+
+=item 'd'
+
+Directory
+
+=item 'f'
+
+FIFO (named pipe)
+
+=item 'l'
+
+Symbolic link
+
+=item 'r'
+
+Regular file
+
+=item 's'
+
+Socket
+
+=item 'h'
+
+Shadow inode (Solaris)
+
+=item 'w'
+
+Whiteout inode (BSD)
+
+=item 'u'
+
+Unknown file type
+
+=back" };
+
 ]

 (* daemon_functions are any functions which cause some action
diff --git a/src/Makefile.am b/src/Makefile.am
index 3b4cd10..9f8af4c 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -130,6 +130,7 @@ libguestfs_la_SOURCES = \
structs-copy.c \
structs-free.c \
tmpdirs.c \
+   tsk.c \
whole-file.c \
libguestfs.syms

diff --git a/src/tsk.c b/src/tsk.c
new file mode 100644
index 000..4d7fd7c
--- /dev/null
+++ b/src/tsk.c
@@ -0,0 +1,123 @@
+/* libguestfs
+ * Copyright (C) 2016 Red Hat Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "full-read.h"
+
+#include "guestfs.h"
+#include "guestfs_protocol.h"
+#include "guestfs-internal.h"
+#include "guestfs-internal-all.h"
+#include "guestfs-internal-actions.h"
+
+static struct guestfs_tsk_dirent_list *parse_filesystem_walk (guestfs_h *, 
char *, size_t );
+int deserialise_dirent_list (guestfs_h *, char *, size_t , struct 
guestfs_tsk_dirent_list *);
+
+struct guestfs_tsk_dirent_list *
+guestfs_impl_filesystem_walk (guestfs_h *g, const char *mountable)
+{
+  int ret = 0;
+  size_t size = 0;
+  CLEANUP_FREE char *buf = NULL;
+  CLEANUP_UNLINK_FREE char *tmpfile = NULL;
+
+  ret = guestfs_int_lazy_make_tmpdir (g);
+  if (ret < 0)
+return NULL;
+
+  tmpfile = safe_asprintf (g, "%s/filesystem_walk%d", g->tmpdir, ++g->unique);
+
+  ret = guestfs_internal_filesystem_walk (g, mountable, tmpfile);
+  if (ret < 0)
+return NULL;
+
+  ret = guestfs_int_read_whole_file (g, tmpfile, , );
+  if (ret < 0)
+return NULL;
+
+  return parse_filesystem_walk (g, buf, size);  /* caller frees */
+}
+
+/* Parse buf content and return dirents list.
+ * Return a list of tsk_dirent on success, NULL on error.
+ */
+static struct guestfs_tsk_dirent_list *
+parse_filesystem_walk (guestfs_h *g, char *buf, size_t bufsize)
+{
+  int ret = 0;
+  struct guestfs_tsk_dirent_list 

[Libguestfs] [PATCH v3 5/5] appliance: Added filesystem_walk command tests

2016-04-05 Thread Matteo Cafasso
The tests check that the filesystem_walk command is able to retrieve
information regarding both existing and deleted files.

A NTFS image is used as Ext3+ filesystems deletion is more aggressive
in terms of metadata removal.

Signed-off-by: Matteo Cafasso 
---
 tests/tsk/Makefile.am |  3 +-
 tests/tsk/test-filesystem-walk.sh | 62 +++
 2 files changed, 64 insertions(+), 1 deletion(-)
 create mode 100755 tests/tsk/test-filesystem-walk.sh

diff --git a/tests/tsk/Makefile.am b/tests/tsk/Makefile.am
index 0cd7c03..f9b2fef 100644
--- a/tests/tsk/Makefile.am
+++ b/tests/tsk/Makefile.am
@@ -18,7 +18,8 @@
 include $(top_srcdir)/subdir-rules.mk

 TESTS = \
-   test-download-inode.sh
+   test-download-inode.sh \
+   test-filesystem-walk.sh

 TESTS_ENVIRONMENT = $(top_builddir)/run --test

diff --git a/tests/tsk/test-filesystem-walk.sh 
b/tests/tsk/test-filesystem-walk.sh
new file mode 100755
index 000..ab7c1a9
--- /dev/null
+++ b/tests/tsk/test-filesystem-walk.sh
@@ -0,0 +1,62 @@
+#!/bin/bash -
+# libguestfs
+# Copyright (C) 2016 Red Hat Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+# Test the filesystem-walk command.
+
+if [ -n "$SKIP_TEST_FILESYSTEM_WALK_SH" ]; then
+echo "$0: test skipped because environment variable is set."
+exit 77
+fi
+
+# Skip if TSK is not supported by the appliance.
+if ! guestfish add /dev/null : run : available "libtsk"; then
+echo "$0: skipped because TSK is not available in the appliance"
+exit 77
+fi
+
+if [ ! -s ../../test-data/phony-guests/windows.img ]; then
+echo "$0: skipped because windows.img is zero-sized"
+exit 77
+fi
+
+# create and delete a file then list the filesystem content
+output=$(guestfish --ro -a ../../test-data/phony-guests/windows.img \
+   run :\
+   mount /dev/sda2 / :  \
+   write /test.txt "foobar" :   \
+   rm /test.txt :   \
+   umount / :   \
+   filesystem-walk /dev/sda2)
+
+# test $MFT is in the list
+echo $output | grep -q "{ tsk_inode: 0 tsk_type: r tsk_size: .* tsk_name: 
\$MFT tsk_allocated: 1 }"
+if [ $? != 0 ]; then
+echo "$0: \$MFT not found in files list."
+echo "File list:"
+echo $output
+exit 1
+fi
+
+# test deleted file is in the list
+echo $output | grep -q "{ tsk_inode: .* tsk_type: [ru] tsk_size: .* tsk_name: 
test.txt tsk_allocated: 0 }"
+if [ $? != 0 ]; then
+echo "$0: /test.txt not found in files list."
+echo "File list:"
+echo $output
+exit 1
+fi
--
2.8.0.rc3

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs


[Libguestfs] [PATCH v3 0/5] Added filesystem_walk command

2016-04-05 Thread Matteo Cafasso
v3:

 - File size will be reported as - 1 if it cannot be retrieved.

 - Code improvements based on comments.

Matteo Cafasso (5):
  generator: Added tsk_dirent struct
  configure: Added libtsk compile-time check
  daemon: Added internal_filesystem_walk command
  appliance: Added filesystem_walk command
  appliance: Added filesystem_walk command tests

 daemon/Makefile.am|   4 +-
 daemon/tsk.c  | 233 ++
 generator/actions.ml  |  78 +
 generator/structs.ml  |  16 ++-
 m4/guestfs_daemon.m4  |   8 ++
 src/MAX_PROC_NR   |   2 +-
 src/Makefile.am   |   1 +
 src/tsk.c | 123 
 tests/tsk/Makefile.am |   3 +-
 tests/tsk/test-filesystem-walk.sh |  62 ++
 10 files changed, 525 insertions(+), 5 deletions(-)
 create mode 100644 daemon/tsk.c
 create mode 100644 src/tsk.c
 create mode 100755 tests/tsk/test-filesystem-walk.sh

--
2.8.0.rc3

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs


[Libguestfs] [PATCH v3 2/5] configure: Added libtsk compile-time check

2016-04-05 Thread Matteo Cafasso
Ensure libtsk is available at compile time.
If not, daemon routines depending on it won't be available.

Signed-off-by: Matteo Cafasso 
---
 m4/guestfs_daemon.m4 | 8 
 1 file changed, 8 insertions(+)

diff --git a/m4/guestfs_daemon.m4 b/m4/guestfs_daemon.m4
index 88936b2..09cfecd 100644
--- a/m4/guestfs_daemon.m4
+++ b/m4/guestfs_daemon.m4
@@ -118,3 +118,11 @@ PKG_CHECK_MODULES([SD_JOURNAL], [libsystemd],[
 AC_MSG_WARN([systemd journal library not found, some features will be 
disabled])
 ])
 ])
+
+dnl libtsk sleuthkit library (optional)
+AC_CHECK_LIB([tsk],[tsk_version_print],[
+AC_CHECK_HEADER([tsk/libtsk.h],[
+AC_SUBST([TSK_LIBS], [-ltsk])
+AC_DEFINE([HAVE_LIBTSK], [1], [Define to 1 if The Sleuth Kit  library 
(libtsk) is available.])
+], [])
+],[AC_MSG_WARN([The Sleuth Kit library (libtsk) not found])])
--
2.8.0.rc3

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs


[Libguestfs] [PATCH v3 3/5] daemon: Added internal_filesystem_walk command

2016-04-05 Thread Matteo Cafasso
The internal_filesystem_walk command walks through the FS structures
of a disk partition and returns all the files or directories
which could be found.

The command is able to retrieve information regarding deleted
or unaccessible files where other commands such as stat or find
would fail.

The gathered list of tsk_dirent structs is serialised into XDR format
and written to a file by the appliance.

Signed-off-by: Matteo Cafasso 
---
 daemon/Makefile.am   |   4 +-
 daemon/tsk.c | 233 +++
 generator/actions.ml |   9 ++
 src/MAX_PROC_NR  |   2 +-
 4 files changed, 246 insertions(+), 2 deletions(-)
 create mode 100644 daemon/tsk.c

diff --git a/daemon/Makefile.am b/daemon/Makefile.am
index beb7962..03bf71f 100644
--- a/daemon/Makefile.am
+++ b/daemon/Makefile.am
@@ -179,6 +179,7 @@ guestfsd_SOURCES = \
sync.c \
syslinux.c \
tar.c \
+   tsk.c \
truncate.c \
umask.c \
upload.c \
@@ -209,7 +210,8 @@ guestfsd_LDADD = \
$(LIB_CLOCK_GETTIME) \
$(LIBINTL) \
$(SERVENT_LIB) \
-   $(PCRE_LIBS)
+   $(PCRE_LIBS) \
+   $(TSK_LIBS)

 guestfsd_CPPFLAGS = \
-I$(top_srcdir)/gnulib/lib \
diff --git a/daemon/tsk.c b/daemon/tsk.c
new file mode 100644
index 000..cd4879b
--- /dev/null
+++ b/daemon/tsk.c
@@ -0,0 +1,233 @@
+/* libguestfs - the guestfsd daemon
+ * Copyright (C) 2016 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 
USA.
+ */
+
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "guestfs_protocol.h"
+#include "daemon.h"
+#include "actions.h"
+#include "optgroups.h"
+
+#ifdef HAVE_LIBTSK
+
+#include 
+
+static int open_filesystem (const char *, TSK_IMG_INFO **, TSK_FS_INFO **);
+static TSK_WALK_RET_ENUM fswalk_callback (TSK_FS_FILE *, const char *, void *);
+static char file_type (TSK_FS_FILE *);
+static int send_dirent_info (guestfs_int_tsk_dirent *);
+static void reply_with_tsk_error (const char *);
+
+int
+do_internal_filesystem_walk (const mountable_t *mountable)
+{
+  int ret = -1;
+  TSK_FS_INFO *fs = NULL;
+  TSK_IMG_INFO *img = NULL;  /* Used internally by tsk_fs_dir_walk */
+  int flags = TSK_FS_DIR_WALK_FLAG_ALLOC | TSK_FS_DIR_WALK_FLAG_UNALLOC |
+TSK_FS_DIR_WALK_FLAG_RECURSE | TSK_FS_DIR_WALK_FLAG_NOORPHAN;
+
+  ret = open_filesystem (mountable->device, , );
+  if (ret < 0)
+return ret;
+
+  reply (NULL, NULL);  /* Reply message. */
+
+  ret = tsk_fs_dir_walk (fs, fs->root_inum, flags, fswalk_callback, NULL);
+  if (ret == 0)
+ret = send_file_end (0);  /* File transfer end. */
+  else
+send_file_end (1);  /* Cancel file transfer. */
+
+  fs->close (fs);
+  img->close (img);
+
+  return ret;
+}
+
+/* Inspect the device and initialises the img and fs structures.
+ * Return 0 on success, -1 on error.
+ */
+static int
+open_filesystem (const char *device, TSK_IMG_INFO **img, TSK_FS_INFO **fs)
+{
+  const char *images[] = { device };
+
+  *img = tsk_img_open (1, images, TSK_IMG_TYPE_DETECT , 0);
+  if (*img == NULL) {
+reply_with_tsk_error ("tsk_image_open");
+return -1;
+  }
+
+  *fs = tsk_fs_open_img (*img, 0, TSK_FS_TYPE_DETECT);
+  if (*fs == NULL) {
+reply_with_tsk_error ("tsk_fs_open_img");
+(*img)->close (*img);
+return -1;
+  }
+
+  return 0;
+}
+
+/* Filesystem walk callback, it gets called on every FS node.
+ * Parse the node, encode it into an XDR structure and send it to the 
appliance.
+ * Return TSK_WALK_CONT on success, TSK_WALK_ERROR on error.
+ */
+static TSK_WALK_RET_ENUM
+fswalk_callback (TSK_FS_FILE *fsfile, const char *path, void *data)
+{
+  int ret = 0;
+  CLEANUP_FREE char *fname = NULL;
+  struct guestfs_int_tsk_dirent dirent;
+
+  /* Ignore ./ and ../ */
+  ret = TSK_FS_ISDOT (fsfile->name->name);
+  if (ret != 0)
+return TSK_WALK_CONT;
+
+  /* Build the full relative path of the entry */
+  ret = asprintf_nowarn (, "%s%s", path, fsfile->name->name);
+  if (ret < 0) {
+fprintf (stderr, "asprintf: %m");
+return TSK_WALK_ERROR;
+  }
+
+  dirent.tsk_inode = fsfile->name->meta_addr;
+  dirent.tsk_type = file_type (fsfile);
+  dirent.tsk_size = (fsfile->meta != NULL) ? fsfile->meta->size : -1;
+  dirent.tsk_name = fname;
+  

[Libguestfs] [PATCH v3 1/5] generator: Added tsk_dirent struct

2016-04-05 Thread Matteo Cafasso
The tsk_dirent struct contains the information gathered via TSK APIs.

The struct contains the following fields:
 * tsk_inode: inode of a file
 * tsk_type: type of file such as for dirwalk command
 * tsk_size: file size in bytes
 * tsk_name: path relative to its disk partition
 * tsk_allocated: whether the file has been deleted

Signed-off-by: Matteo Cafasso 
---
 generator/structs.ml | 16 ++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/generator/structs.ml b/generator/structs.ml
index 6017ba6..d986fd9 100644
--- a/generator/structs.ml
+++ b/generator/structs.ml
@@ -442,8 +442,20 @@ let structs = [
 "im_device", FString;
 "im_volume", FString;
 ];
-s_camel_name = "InternalMountable";
-  };
+s_camel_name = "InternalMountable" };
+
+  (* The Sleuth Kit directory entry information. *)
+  { defaults with
+s_name = "tsk_dirent";
+s_cols = [
+"tsk_inode", FUInt64;
+"tsk_type", FChar;
+"tsk_size", FInt64;
+"tsk_name", FString;
+"tsk_allocated", FUInt32;
+];
+s_camel_name = "TSKDirent" };
+
 ] (* end of structs *)

 let lookup_struct name =
--
2.8.0.rc3

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs


Re: [Libguestfs] [PATCH 7/7] v2v: add support for SUSE VMDP drivers

2016-04-05 Thread Cedric Bosdonnat
Here are the answers from our VMDP expert:

On Tue, 2016-04-05 at 16:16 +0300, Roman Kagan wrote:
> On Tue, Apr 05, 2016 at 03:08:48PM +0200, Cedric Bosdonnat wrote:
> > On Tue, 2016-04-05 at 15:52 +0300, Roman Kagan wrote:
> > > On Tue, Apr 05, 2016 at 01:47:33PM +0200, Cédric Bosdonnat wrote:
> > > > Note that VMDP's block driver pvvxblk depends on the ballooning
> > > > driver
> > > > pvvxbn.
> > > 
> > > Can you please elaborate on this?  Is the balloon driver *used*
> > > by
> > > the
> > > storage driver so that the latter won't work without the former? 

That's correct.  The current design requires that the balloon driver be
loaded. The pvvxblk driver is a single binary that runs on both KVM and
Xen.  It determines at runtime which environment it is running on and
acts accordingly.  The dependency is required for Xen.  This also
creates the dependency for KVM. 

> > >  What
> > > will happen if the virtio-balloon device isn't configured in the 
> > > VM?

The balloon driver will not load and hence the block driver will not
load.

> > It's just that without the virtio balloon driver installed, the
> > virtio
> > block one won't start properly, resulting in the famous 0x7B error.
> 
> Does it also require the presence of the balloon device?

Yes.  This the default installation.  The only way to remove the
balloon device is to launch the vm by constructing a manually
configured command line.  Weather the balloon device is present or not,
the user is able to set the current and maximum memory allocations
differently.  If different and the balloon device/driver is not
present, the vm will undoubtedly crash at some point. 

--
Cedric

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH 6/7] v2v: quiet virtio net and balloon devices wizards

2016-04-05 Thread Cedric Bosdonnat
On Tue, 2016-04-05 at 17:19 +0300, Roman Kagan wrote:
> On Tue, Apr 05, 2016 at 01:47:32PM +0200, Cédric Bosdonnat wrote:
> > Setting the ConfigFlags to 0x40 for those will make windows quiet
> > at the first boot about those new devices. The wizard must not be
> > presented to the user since the needed drivers will automatically
> > be installed at firstboot... or worse, the wizard can even block
> > the installer.
> 
> What installer?

At least the VMDP installer running at firstboot is blocked by these
wizards.

> You're trying circumvent the usual PnP process people are used to. 
>  I'm
> not sure it's worth adding yet more unreliable hacks (yes, basically
> the
> whole v2v/windows_virtio.ml is a hack).

Setting those keys forces windows to ignore the virtio net and balloon
devices for the first boot time. Running the VMDP installer (or the RH
equivalent one) will install the needed drivers and all will be fine
after that.

> > ---
> >  v2v/windows_virtio.ml | 8 
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/v2v/windows_virtio.ml b/v2v/windows_virtio.ml
> > index dfb7b71..22e3e31 100644
> > --- a/v2v/windows_virtio.ml
> > +++ b/v2v/windows_virtio.ml
> > @@ -196,6 +196,14 @@ and add_viostor_to_critical_device_database g
> > root current_cs major =
> >[ "0", REG_SZ (sprintf
> > "PCI\\VEN_1AF4_1001_00021AF4_00\\%s&20" subkey);
> >  "Count", REG_DWORD 0x1_l;
> >  "NextInstance", REG_DWORD 0x1_l ];
> > +
> > +  [ current_cs; "Enum"; "PCI";
> > "VEN_1AF4_1000_00011AF4_00"; subkey ^ "&18" ],
> > +  [ "ConfigFlags", REG_DWORD 0x40_l ];
> > +  [ current_cs; "Enum"; "PCI";
> > "VEN_1AF4_1001_00021AF4_00"; subkey ^ "&20" ],
> > +  [ "ConfigFlags", REG_DWORD 0x0_l;
> > +"Service", REG_SZ driver_name ];
> > +  [ current_cs; "Enum"; "PCI";
> > "VEN_1AF4_1002_00051AF4_00"; subkey ^ "&28" ],
> > +  [ "ConfigFlags", REG_DWORD 0x40_l ];
> 
> I'm curious how reliable those keys are; what are the chances that
> the
> devices get assigned different instance ids?  I couldn't find any
> sources indicating that those instance ids are assigned in any
> predictable manner.

I have no idea how they are computed, but they are common accross all
windows versions except for the subkey that changes with windows
versions.

I tried to get rid of those subkeys, but then windows keeps showing the
wizards without those.

--
Cedric

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH 2/7] v2v: extract controller offset discovery as a function

2016-04-05 Thread Roman Kagan
On Tue, Apr 05, 2016 at 01:47:28PM +0200, Cédric Bosdonnat wrote:
> This function is needed for other drivers, move the code in order to
> help sharing it later.

In my experiments this key isn't necessary at all, so I was about to
nuke it altogether.

Roman.

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs


Re: [Libguestfs] [PATCH 6/7] v2v: quiet virtio net and balloon devices wizards

2016-04-05 Thread Roman Kagan
On Tue, Apr 05, 2016 at 01:47:32PM +0200, Cédric Bosdonnat wrote:
> Setting the ConfigFlags to 0x40 for those will make windows quiet
> at the first boot about those new devices. The wizard must not be
> presented to the user since the needed drivers will automatically
> be installed at firstboot... or worse, the wizard can even block
> the installer.

What installer?

You're trying circumvent the usual PnP process people are used to.  I'm
not sure it's worth adding yet more unreliable hacks (yes, basically the
whole v2v/windows_virtio.ml is a hack).

> ---
>  v2v/windows_virtio.ml | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/v2v/windows_virtio.ml b/v2v/windows_virtio.ml
> index dfb7b71..22e3e31 100644
> --- a/v2v/windows_virtio.ml
> +++ b/v2v/windows_virtio.ml
> @@ -196,6 +196,14 @@ and add_viostor_to_critical_device_database g root 
> current_cs major =
>[ "0", REG_SZ (sprintf 
> "PCI\\VEN_1AF4_1001_00021AF4_00\\%s&20" subkey);
>  "Count", REG_DWORD 0x1_l;
>  "NextInstance", REG_DWORD 0x1_l ];
> +
> +  [ current_cs; "Enum"; "PCI"; 
> "VEN_1AF4_1000_00011AF4_00"; subkey ^ "&18" ],
> +  [ "ConfigFlags", REG_DWORD 0x40_l ];
> +  [ current_cs; "Enum"; "PCI"; 
> "VEN_1AF4_1001_00021AF4_00"; subkey ^ "&20" ],
> +  [ "ConfigFlags", REG_DWORD 0x0_l;
> +"Service", REG_SZ driver_name ];
> +  [ current_cs; "Enum"; "PCI"; 
> "VEN_1AF4_1002_00051AF4_00"; subkey ^ "&28" ],
> +  [ "ConfigFlags", REG_DWORD 0x40_l ];

I'm curious how reliable those keys are; what are the chances that the
devices get assigned different instance ids?  I couldn't find any
sources indicating that those instance ids are assigned in any
predictable manner.

Roman.

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs


Re: [Libguestfs] [PATCH 7/7] v2v: add support for SUSE VMDP drivers

2016-04-05 Thread Roman Kagan
On Tue, Apr 05, 2016 at 03:08:48PM +0200, Cedric Bosdonnat wrote:
> On Tue, 2016-04-05 at 15:52 +0300, Roman Kagan wrote:
> > On Tue, Apr 05, 2016 at 01:47:33PM +0200, Cédric Bosdonnat wrote:
> > > Note that VMDP's block driver pvvxblk depends on the ballooning
> > > driver
> > > pvvxbn.
> > 
> > Can you please elaborate on this?  Is the balloon driver *used* by
> > the
> > storage driver so that the latter won't work without the former? 
> >  What
> > will happen if the virtio-balloon device isn't configured in the VM?
> 
> It's just that without the virtio balloon driver installed, the virtio
> block one won't start properly, resulting in the famous 0x7B error.

Does it also require the presence of the balloon device?

Roman.

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs


Re: [Libguestfs] [PATCH 7/7] v2v: add support for SUSE VMDP drivers

2016-04-05 Thread Cedric Bosdonnat
On Tue, 2016-04-05 at 15:52 +0300, Roman Kagan wrote:
> On Tue, Apr 05, 2016 at 01:47:33PM +0200, Cédric Bosdonnat wrote:
> > To add this support, the existing code searches for either the
> > viostor
> > or the VMDP (Virtual Machine Driver Pack) files and updates the
> > registry
> > accordingly.
> > 
> > Note that VMDP's block driver pvvxblk depends on the ballooning
> > driver
> > pvvxbn.
> 
> Can you please elaborate on this?  Is the balloon driver *used* by
> the
> storage driver so that the latter won't work without the former? 
>  What
> will happen if the virtio-balloon device isn't configured in the VM?

It's just that without the virtio balloon driver installed, the virtio
block one won't start properly, resulting in the famous 0x7B error.

> Or is it just a matter of ordering of the balloon driver within the
> driver start sequence, so that it can get its hands on the system as
> early as possible for its own purposes, and the storage driver
> doesn't
> require balloon's presence?

It's not ordering, it really requires it.

--
Cedric

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH 7/7] v2v: add support for SUSE VMDP drivers

2016-04-05 Thread Roman Kagan
On Tue, Apr 05, 2016 at 01:47:33PM +0200, Cédric Bosdonnat wrote:
> To add this support, the existing code searches for either the viostor
> or the VMDP (Virtual Machine Driver Pack) files and updates the registry
> accordingly.
> 
> Note that VMDP's block driver pvvxblk depends on the ballooning driver
> pvvxbn.

Can you please elaborate on this?  Is the balloon driver *used* by the
storage driver so that the latter won't work without the former?  What
will happen if the virtio-balloon device isn't configured in the VM?

Or is it just a matter of ordering of the balloon driver within the
driver start sequence, so that it can get its hands on the system as
early as possible for its own purposes, and the storage driver doesn't
require balloon's presence?

Thanks,
Roman.

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs


Re: [Libguestfs] [PATCH 3/7] customize: add support for pvvxsvc

2016-04-05 Thread Cedric Bosdonnat
On Tue, 2016-04-05 at 13:11 +0100, Richard W.M. Jones wrote:
> On Tue, Apr 05, 2016 at 01:47:29PM +0200, Cédric Bosdonnat wrote:
> >  The output of the first boot scripts is available in the guest as
> > -F.
> > +F.
> 
> Changing this path is fine, but let's split it into a separate patch.

Ok, I'll split that one.

> > +let services = ["rhsrvany.exe"; "pvvxsvc.exe"] in
> > +let srvany = (
> > +  try
> > +List.find (
> > +  fun service -> (
> > +try
> > +  let chan = open_in (virt_tools_data_dir // service)
> > in
> > +  close_in chan;
> > +  true
> > +with _ ->
> > +  false
> > +  )
> > +) services
> > +  with Not_found ->
> > +   error (f_"One of rhsrvany.exe or pvvxsvc.exe is missing in
> > %s.  One of them is required in order to install Windows firstboot
> > scripts.  You can get one by building rhsrvany (
> > https://github.com/rwmjones/rhsrvany)")
> > + virt_tools_data_dir
> > +) in (
> 
> There's a stray ( here, and lots of code gets reindented
> for reasons I don't understand but may be connected to that
> stray (.

Oh, I think that's an ocaml-beginner problem of mine ;) I'll fix it.

--
Cedric

> > +  g#hivex_commit None;
> > +  g#hivex_close ();
> > +
> > +  firstboot_dir
> > +)
> 
> End of the stray ) and reformatting.
> 
> Rich.
> 

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH 1/7] v2v: check next free oem%d.inf in /Windows/Inf

2016-04-05 Thread Cedric Bosdonnat
On Tue, 2016-04-05 at 13:04 +0100, Richard W.M. Jones wrote:
> On Tue, Apr 05, 2016 at 01:47:27PM +0200, Cédric Bosdonnat wrote:
> > +  let oem_inf = set_free_oem_inf g root scsi_adapter_guid
> > "viostor.inf" driverdir in
> 
> Seems better if it was called *get_next*_free_oem_inf?

Yes, sounds better.

> >(* There should be a key
> > *   HKLM\SYSTEM\ControlSet001\Control\Class\
> > @@ -398,6 +378,28 @@ and add_viostor_to_driver_database g root arch
> > current_cs =
> >
> >  @=hex(0012):6f,00,65,00,6d,00,31,00,2e,00,69,00,6e,00,66,00,00
> > ,00
> >  *)
> >  
> > +(* There should be a key
> > + *   HKLM\SYSTEM\DriverDatabase\DeviceIds\
> > + * We want to add:
> > + *   "oem1.inf"=hex(0):
> > + * but if we find "oem1.inf" we'll add "oem2.inf" (etc).
> > + *)
> > +and set_free_oem_inf g root guid driver_inf driverdir =
> > +  let path = [ "DriverDatabase"; "DeviceIds"; guid ] in
> > +  match Windows.get_node g root path with
> > +  | None ->
> > + error (f_"cannot find
> > HKLM\\SYSTEM\\DriverDatabase\\DeviceIds\\%s in the guest registry")
> > guid
> > +  | Some node ->
> > +let rec loop i =
> > +  let oem_inf = sprintf "oem%d.inf" i in
> > +  if not (g#exists ("/Windows/Inf/" ^ oem_inf)) then oem_inf
> > else loop (i+1)
> > +in
> 
> This bit doesn't match what is described in the comment.  It's also
> incorrect for a few reasons:

May be I should add to the comment that it searches for the next
available oem%d.inf in the Windows\Inf folder?

>  - It should use windows_systemroot, instead of "/Windows"
> 
>  - It doesn't handle case sensitive path stuff

Indeed, I'll need to fix those

> Do we still need to check for the registry key?  (I have no idea)

As we add the key just a few lines below, we need to check for it


> > +let oem_inf = loop 1 in
> > +(* Create the key. *)
> > +g#hivex_node_set_value node oem_inf (* REG_NONE *) 0_L "";
> > +g#cp (driverdir // driver_inf) ("/Windows/Inf/" ^ oem_inf);
> 
> And this line seems like a bit of a hack.  We have a place where
> drivers are copied into the driverdir.  I think it would be better if
> we returned oem_inf from add_viostor_to_registry. 
>  But ...

It's not exactly in driverdir, it's in a subfolder of it. The problem
is that it seems that Windows gets the next available oem%d.inf from
the files in /Windows/Inf... if we aren't copying them manually while
we reserve it, we end up with conflicts when we have more than one
driver to add.

I don't see why you say you'ld see oem_inf returned from
 add_viostor_to_registry: we don't need it outside that function,
right?

> Do we actually need to do this copy at all?  The Red Hat drivers
> don't
> require this, in order to boot (note that with the Red Hat drivers we
> only half install them, they are properly installed when Windows
> boots).

The virtio block driver from SUSE VMDP has a dependency on the balloon
driver. That's why we need to half install these two before the first
boot.

The situation is cleaned up at first boot and the other drivers are
installed at that time too.

--
Cedric

> Rich.
> 
> > +oem_inf
> > +
> >  (* Copy the matching drivers to the driverdir; return true if any
> > have
> >   * been copied.
> >   *)
> > -- 
> > 2.6.2
> > 
> > ___
> > Libguestfs mailing list
> > Libguestfs@redhat.com
> > https://www.redhat.com/mailman/listinfo/libguestfs
> 

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH 1/7] v2v: check next free oem%d.inf in /Windows/Inf

2016-04-05 Thread Cedric Bosdonnat
On Tue, 2016-04-05 at 15:13 +0300, Roman Kagan wrote:
> On Tue, Apr 05, 2016 at 01:47:27PM +0200, Cédric Bosdonnat wrote:
> > It seems that checking for oem%d.inf in the DeviceIds registry
> > entry
> > doesn't always list all oemXX.inf files. For example we may have
> > oem1.inf free in the registry key, but used in another one.
> 
> In my experiments this name doesn't have to bear any sense, so
> there's
> no point doing anything smart about it.

What I did was not a strictly scientific approach, rather trying to
mimic what seemed to happen ;)

> FWIW I've been working on an alternative apporoach (mostly about
> reducing the amount of regedits) and was about to submit the patches
> today.  I guess it'd accomodate SUSE driver suite just as well.

Cool, I'ld be happy to test your patches together with mines ;)

--
Cedric

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH 1/7] v2v: check next free oem%d.inf in /Windows/Inf

2016-04-05 Thread Richard W.M. Jones
On Tue, Apr 05, 2016 at 02:24:33PM +0200, Cedric Bosdonnat wrote:
> On Tue, 2016-04-05 at 13:04 +0100, Richard W.M. Jones wrote:
> > On Tue, Apr 05, 2016 at 01:47:27PM +0200, Cédric Bosdonnat wrote:
> > > +  let oem_inf = set_free_oem_inf g root scsi_adapter_guid
> > > "viostor.inf" driverdir in
> > 
> > Seems better if it was called *get_next*_free_oem_inf?
> 
> Yes, sounds better.
> 
> > >(* There should be a key
> > > *   HKLM\SYSTEM\ControlSet001\Control\Class\
> > > @@ -398,6 +378,28 @@ and add_viostor_to_driver_database g root arch
> > > current_cs =
> > >
> > >  @=hex(0012):6f,00,65,00,6d,00,31,00,2e,00,69,00,6e,00,66,00,00
> > > ,00
> > >  *)
> > >  
> > > +(* There should be a key
> > > + *   HKLM\SYSTEM\DriverDatabase\DeviceIds\
> > > + * We want to add:
> > > + *   "oem1.inf"=hex(0):
> > > + * but if we find "oem1.inf" we'll add "oem2.inf" (etc).
> > > + *)
> > > +and set_free_oem_inf g root guid driver_inf driverdir =
> > > +  let path = [ "DriverDatabase"; "DeviceIds"; guid ] in
> > > +  match Windows.get_node g root path with
> > > +  | None ->
> > > + error (f_"cannot find
> > > HKLM\\SYSTEM\\DriverDatabase\\DeviceIds\\%s in the guest registry")
> > > guid
> > > +  | Some node ->
> > > +let rec loop i =
> > > +  let oem_inf = sprintf "oem%d.inf" i in
> > > +  if not (g#exists ("/Windows/Inf/" ^ oem_inf)) then oem_inf
> > > else loop (i+1)
> > > +in
> > 
> > This bit doesn't match what is described in the comment.  It's also
> > incorrect for a few reasons:
> 
> May be I should add to the comment that it searches for the next
> available oem%d.inf in the Windows\Inf folder?

That would seem to be more accurate.

On the other points, I'd like to see what Roman suggests.  Maybe
we can just make up a random name instead of using oemNN.inf?

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs


Re: [Libguestfs] [PATCH 7/7] v2v: add support for SUSE VMDP drivers

2016-04-05 Thread Richard W.M. Jones
On Tue, Apr 05, 2016 at 01:47:33PM +0200, Cédric Bosdonnat wrote:
> To add this support, the existing code searches for either the viostor
> or the VMDP (Virtual Machine Driver Pack) files and updates the registry
> accordingly.
> 
> Note that VMDP's block driver pvvxblk depends on the ballooning driver
> pvvxbn.

I didn't check this in detail, but it seems reasonable.

Rich.

>  v2v/convert_windows.ml |  59 --
>  v2v/windows_virtio.ml  | 113 
> +
>  2 files changed, 141 insertions(+), 31 deletions(-)
> 
> diff --git a/v2v/convert_windows.ml b/v2v/convert_windows.ml
> index 5daae6c..cb936ec 100644
> --- a/v2v/convert_windows.ml
> +++ b/v2v/convert_windows.ml
> @@ -43,18 +43,25 @@ let convert ~keep_serial_console (g : G.guestfs) inspect 
> source rcaps =
>  try Sys.getenv "VIRT_TOOLS_DATA_DIR"
>  with Not_found -> Guestfs_config.datadir // "virt-tools" in
>  
> -  (* Check if RHEV-APT exists.  This is optional. *)
> -  let rhev_apt_exe = virt_tools_data_dir // "rhev-apt.exe" in
> -  let rhev_apt_exe =
> +  (* Check if either RHEV-APT or VMDP exists.  This is optional. *)
> +  let tools = ["rhev-apt.exe"; "vmdp.exe"] in
> +  let installer =
>  try
> -  let chan = open_in rhev_apt_exe in
> -  close_in chan;
> -  Some rhev_apt_exe
> -with
> -  Sys_error msg ->
> -warning (f_"'%s' is missing.  Unable to install RHEV-APT (RHEV guest 
> agent).  Original error: %s")
> -  rhev_apt_exe msg;
> -None in
> +  let tool = List.find (
> +fun item ->
> +  try (
> +let exe_path = virt_tools_data_dir // item in
> +let chan = open_in exe_path in
> +close_in chan;
> +true
> +  ) with _ ->
> +false
> +  ) tools in
> +  Some (virt_tools_data_dir // tool)
> +with Not_found -> (
> +  warning (f_"Neither rhev-apt.exe nor vmdp.exe can be found.  Unable to 
> install one of them.");
> +  None
> +) in
>  
>(* Get the Windows %systemroot%. *)
>let systemroot = g#inspect_get_windows_systemroot inspect.i_root in
> @@ -211,7 +218,14 @@ let convert ~keep_serial_console (g : G.guestfs) inspect 
> source rcaps =
>(* Perform the conversion of the Windows guest. *)
>  
>let rec configure_firstboot () =
> -configure_rhev_apt ();
> +match installer with
> +| None -> info (f_"No firstboot installer to configure")
> +| Some installer_path ->
> +   let installer_name = Filename.basename installer_path in
> +   match installer_name with
> +| "rhev-apt.exe" -> configure_rhev_apt ()
> +| "vmdp.exe" -> configure_vmdp ()
> +| _ -> info (f_"No setup function for installer '%s'") 
> installer_path;
>  unconfigure_xenpv ();
>  unconfigure_prltools ()
>  
> @@ -219,7 +233,7 @@ let convert ~keep_serial_console (g : G.guestfs) inspect 
> source rcaps =
>  (* Configure RHEV-APT (the RHEV guest agent).  However if it doesn't
>   * exist just warn about it and continue.
>   *)
> -match rhev_apt_exe with
> +match installer with
>  | None -> ()
>  | Some rhev_apt_exe ->
>g#upload rhev_apt_exe "/rhev-apt.exe"; (* XXX *)
> @@ -236,6 +250,25 @@ net start rhev-apt
>Firstboot.add_firstboot_script g inspect.i_root
>  "configure rhev-apt" fb_script
>  
> +  and configure_vmdp () =
> +(* Configure VMDP if possible *)
> +match installer with
> +| None -> ()
> +| Some vmdp_exe ->
> +  g#upload vmdp_exe "/vmdp.exe";
> +
> +  let fb_script = "\
> +echo V2V first boot script started
> +echo Decompressing VMDP installer
> +\"\\vmdp.exe\"
> +cd \"VMDP-WIN*\"
> +echo Installing VMDP
> +setup.exe /eula_accepted /auto_reboot
> +cd ..
> +" in
> +  Firstboot.add_firstboot_script g inspect.i_root
> +"configure vmdp" fb_script
> +
>and unconfigure_xenpv () =
>  match xenpv_uninst with
>  | None -> () (* nothing to be uninstalled *)
> diff --git a/v2v/windows_virtio.ml b/v2v/windows_virtio.ml
> index 22e3e31..87e39e6 100644
> --- a/v2v/windows_virtio.ml
> +++ b/v2v/windows_virtio.ml
> @@ -62,15 +62,23 @@ let rec install_drivers g inspect systemroot root 
> current_cs rcaps =
>else (
>  (* Can we install the block driver? *)
>  let block : guestcaps_block_type =
> -  let has_viostor = g#exists (driverdir // "viostor.inf") in
> +  let filenames = ["pvvxblk.sys"; "virtio_blk.sys"; "vrtioblk.sys"; 
> "viostor.sys"] in
> +  let driver_name = try (
> +List.find (
> +  fun driver_file ->
> +let source = driverdir // driver_file in
> +g#exists source
> +) filenames
> +  ) with Not_found -> "" in
> +  let has_viostor = not (driver_name = "") in
>match rcaps.rcaps_block_bus, has_viostor with
>| Some Virtio_blk, false ->
> -error (f_"there is no viostor (virtio block device) driver for 

Re: [Libguestfs] [PATCH 6/7] v2v: quiet virtio net and balloon devices wizards

2016-04-05 Thread Richard W.M. Jones
On Tue, Apr 05, 2016 at 01:47:32PM +0200, Cédric Bosdonnat wrote:
> Setting the ConfigFlags to 0x40 for those will make windows quiet
> at the first boot about those new devices. The wizard must not be
> presented to the user since the needed drivers will automatically
> be installed at firstboot... or worse, the wizard can even block
> the installer.
> ---
>  v2v/windows_virtio.ml | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/v2v/windows_virtio.ml b/v2v/windows_virtio.ml
> index dfb7b71..22e3e31 100644
> --- a/v2v/windows_virtio.ml
> +++ b/v2v/windows_virtio.ml
> @@ -196,6 +196,14 @@ and add_viostor_to_critical_device_database g root 
> current_cs major =
>[ "0", REG_SZ (sprintf 
> "PCI\\VEN_1AF4_1001_00021AF4_00\\%s&20" subkey);
>  "Count", REG_DWORD 0x1_l;
>  "NextInstance", REG_DWORD 0x1_l ];
> +
> +  [ current_cs; "Enum"; "PCI"; 
> "VEN_1AF4_1000_00011AF4_00"; subkey ^ "&18" ],
> +  [ "ConfigFlags", REG_DWORD 0x40_l ];
> +  [ current_cs; "Enum"; "PCI"; 
> "VEN_1AF4_1001_00021AF4_00"; subkey ^ "&20" ],
> +  [ "ConfigFlags", REG_DWORD 0x0_l;
> +"Service", REG_SZ driver_name ];
> +  [ current_cs; "Enum"; "PCI"; 
> "VEN_1AF4_1002_00051AF4_00"; subkey ^ "&28" ],
> +  [ "ConfigFlags", REG_DWORD 0x40_l ];
>  ] in

Seems reasonable.

We can probably pull this patch earlier so we can apply it now -
however it needs driver_name changed back to "viostor".

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs


Re: [Libguestfs] [PATCH 5/7] v2v: adapt the subkey in Enum registry to windows version

2016-04-05 Thread Richard W.M. Jones
On Tue, Apr 05, 2016 at 01:47:31PM +0200, Cédric Bosdonnat wrote:
> We need to adapt the 
> Services\viostor\Enum\PCI\VEN_1AF4_1001_00021AF4_00 subkey
> to what windows actually uses.
> ---
>  v2v/windows_virtio.ml | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/v2v/windows_virtio.ml b/v2v/windows_virtio.ml
> index 8a0b529..dfb7b71 100644
> --- a/v2v/windows_virtio.ml
> +++ b/v2v/windows_virtio.ml
> @@ -139,15 +139,18 @@ and add_viostor_to_registry g inspect root current_cs 
> driverdir =
>if (major == 6 && minor >= 2) || major >= 7 then (* Windows >= 8 *)
>  add_viostor_to_driver_database g root arch current_cs driverdir
>else  (* Windows <= 7 *)
> -add_viostor_to_critical_device_database g root current_cs
> +add_viostor_to_critical_device_database g root current_cs major

This is OK, but maybe we can just pass the 'inspect' struct down into
add_viostor_to_critical_device_database (between g & root parameters)?
Maybe we'll need to use other inspection data in future.

We really ought to collect all these parameters into a struct :-)

Rich.

> -and add_viostor_to_critical_device_database g root current_cs =
> +and add_viostor_to_critical_device_database g root current_cs major =
>(* See 
> http://rwmj.wordpress.com/2010/04/30/tip-install-a-device-driver-in-a-windows-vm/
> * NB: All these edits are in the HKLM\SYSTEM hive.  No other
> * hive may be modified here.
> *)
>let driver = "viostor.sys" in
>let driver_name = Filename.chop_extension driver in
> +  (* Windows 2k3 uses '&0&', windows 2k8 '&2&' *)
> +  let subkey =
> +if (major == 5) then "3&13c0b0c5&0" else "3&13c0b0c5&2" in
>let regedits = [
>[ current_cs; "Control"; "CriticalDeviceDatabase"; 
> "pci#ven_1af4_1001_" ],
>[ "Service", REG_SZ driver_name;
> @@ -190,7 +193,7 @@ and add_viostor_to_critical_device_database g root 
> current_cs =
>[ "5", REG_DWORD 0x1_l ];
>  
>[ current_cs; "Services"; driver_name; "Enum" ],
> -  [ "0", REG_SZ 
> "PCI\\VEN_1AF4_1001_00021AF4_00\\3&13c0b0c5&0&20";
> +  [ "0", REG_SZ (sprintf 
> "PCI\\VEN_1AF4_1001_00021AF4_00\\%s&20" subkey);
>  "Count", REG_DWORD 0x1_l;
>  "NextInstance", REG_DWORD 0x1_l ];
>  ] in
> -- 
> 2.6.2
> 
> ___
> Libguestfs mailing list
> Libguestfs@redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs


Re: [Libguestfs] [PATCH 4/7] v2v: extract reusable parts of viostor regedits

2016-04-05 Thread Richard W.M. Jones
On Tue, Apr 05, 2016 at 01:47:30PM +0200, Cédric Bosdonnat wrote:
> There are registry entries that are needed to add some other drivers.
> Extracting them into a function will help adding SUSE VMDP support.

I didn't check this one in detail yet, but it seems reasonable.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs


Re: [Libguestfs] [PATCH 2/7] v2v: extract controller offset discovery as a function

2016-04-05 Thread Richard W.M. Jones
On Tue, Apr 05, 2016 at 01:47:28PM +0200, Cédric Bosdonnat wrote:
> This function is needed for other drivers, move the code in order to
> help sharing it later.

Simple refactoring, looks fine.

Rich.

>  v2v/windows_virtio.ml | 26 ++
>  1 file changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/v2v/windows_virtio.ml b/v2v/windows_virtio.ml
> index b0d9d08..14ffc51 100644
> --- a/v2v/windows_virtio.ml
> +++ b/v2v/windows_virtio.ml
> @@ -226,18 +226,7 @@ and add_viostor_to_driver_database g root arch 
> current_cs driverdir =
> *)
>let controller_path =
>  [ current_cs; "Control"; "Class"; scsi_adapter_guid ] in
> -  let controller_offset =
> -match Windows.get_node g root controller_path with
> -| None ->
> -   error (f_"cannot find HKLM\\SYSTEM\\%s in the guest registry")
> - (String.concat "\\" controller_path)
> -| Some node ->
> -   let rec loop node i =
> - let controller_offset = sprintf "%04d" i in
> - let child = g#hivex_node_get_child node controller_offset in
> - if child = 0_L then controller_offset else loop node (i+1)
> -   in
> -   loop node 0 in
> +  let controller_offset = get_controller_offset g root controller_path in
>  
>let regedits = [
>controller_path @ [ controller_offset ],
> @@ -400,6 +389,19 @@ and set_free_oem_inf g root guid driver_inf driverdir =
>  g#cp (driverdir // driver_inf) ("/Windows/Inf/" ^ oem_inf);
>  oem_inf
>  
> +and get_controller_offset g root controller_path =
> +  match Windows.get_node g root controller_path with
> +  | None ->
> + error (f_"cannot find HKLM\\SYSTEM\\%s in the guest registry")
> +   (String.concat "\\" controller_path)
> +  | Some node ->
> + let rec loop node i =
> +   let controller_offset = sprintf "%04d" i in
> +   let child = g#hivex_node_get_child node controller_offset in
> +   if child = 0_L then controller_offset else loop node (i+1)
> + in
> + loop node 0
> +
>  (* Copy the matching drivers to the driverdir; return true if any have
>   * been copied.
>   *)
> -- 
> 2.6.2
> 
> ___
> Libguestfs mailing list
> Libguestfs@redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs


Re: [Libguestfs] [PATCH 1/7] v2v: check next free oem%d.inf in /Windows/Inf

2016-04-05 Thread Roman Kagan
On Tue, Apr 05, 2016 at 01:47:27PM +0200, Cédric Bosdonnat wrote:
> It seems that checking for oem%d.inf in the DeviceIds registry entry
> doesn't always list all oemXX.inf files. For example we may have
> oem1.inf free in the registry key, but used in another one.

In my experiments this name doesn't have to bear any sense, so there's
no point doing anything smart about it.

FWIW I've been working on an alternative apporoach (mostly about
reducing the amount of regedits) and was about to submit the patches
today.  I guess it'd accomodate SUSE driver suite just as well.

Roman.

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs


Re: [Libguestfs] [PATCH 3/7] customize: add support for pvvxsvc

2016-04-05 Thread Richard W.M. Jones
On Tue, Apr 05, 2016 at 01:47:29PM +0200, Cédric Bosdonnat wrote:
>  The output of the first boot scripts is available in the guest as
> -F.
> +F.

Changing this path is fine, but let's split it into a separate patch.

> +let services = ["rhsrvany.exe"; "pvvxsvc.exe"] in
> +let srvany = (
> +  try
> +List.find (
> +  fun service -> (
> +try
> +  let chan = open_in (virt_tools_data_dir // service) in
> +  close_in chan;
> +  true
> +with _ ->
> +  false
> +  )
> +) services
> +  with Not_found ->
> +   error (f_"One of rhsrvany.exe or pvvxsvc.exe is missing in %s.  One 
> of them is required in order to install Windows firstboot scripts.  You can 
> get one by building rhsrvany (https://github.com/rwmjones/rhsrvany)")
> + virt_tools_data_dir
> +) in (

There's a stray ( here, and lots of code gets reindented
for reasons I don't understand but may be connected to that
stray (.

> +  g#hivex_commit None;
> +  g#hivex_close ();
> +
> +  firstboot_dir
> +)

End of the stray ) and reformatting.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs


Re: [Libguestfs] [PATCH 1/7] v2v: check next free oem%d.inf in /Windows/Inf

2016-04-05 Thread Richard W.M. Jones
On Tue, Apr 05, 2016 at 01:47:27PM +0200, Cédric Bosdonnat wrote:
> +  let oem_inf = set_free_oem_inf g root scsi_adapter_guid "viostor.inf" 
> driverdir in

Seems better if it was called *get_next*_free_oem_inf?

>  
>(* There should be a key
> *   HKLM\SYSTEM\ControlSet001\Control\Class\
> @@ -398,6 +378,28 @@ and add_viostor_to_driver_database g root arch 
> current_cs =
> @=hex(0012):6f,00,65,00,6d,00,31,00,2e,00,69,00,6e,00,66,00,00,00
>  *)
>  
> +(* There should be a key
> + *   HKLM\SYSTEM\DriverDatabase\DeviceIds\
> + * We want to add:
> + *   "oem1.inf"=hex(0):
> + * but if we find "oem1.inf" we'll add "oem2.inf" (etc).
> + *)
> +and set_free_oem_inf g root guid driver_inf driverdir =
> +  let path = [ "DriverDatabase"; "DeviceIds"; guid ] in
> +  match Windows.get_node g root path with
> +  | None ->
> + error (f_"cannot find HKLM\\SYSTEM\\DriverDatabase\\DeviceIds\\%s in 
> the guest registry") guid
> +  | Some node ->
> +let rec loop i =
> +  let oem_inf = sprintf "oem%d.inf" i in
> +  if not (g#exists ("/Windows/Inf/" ^ oem_inf)) then oem_inf else loop 
> (i+1)
> +in

This bit doesn't match what is described in the comment.  It's also
incorrect for a few reasons:

 - It should use windows_systemroot, instead of "/Windows"

 - It doesn't handle case sensitive path stuff

Do we still need to check for the registry key?  (I have no idea)

> +let oem_inf = loop 1 in
> +(* Create the key. *)
> +g#hivex_node_set_value node oem_inf (* REG_NONE *) 0_L "";
> +g#cp (driverdir // driver_inf) ("/Windows/Inf/" ^ oem_inf);

And this line seems like a bit of a hack.  We have a place where
drivers are copied into the driverdir.  I think it would be better if
we returned oem_inf from add_viostor_to_registry.  But ...

Do we actually need to do this copy at all?  The Red Hat drivers don't
require this, in order to boot (note that with the Red Hat drivers we
only half install them, they are properly installed when Windows
boots).

Rich.

> +oem_inf
> +
>  (* Copy the matching drivers to the driverdir; return true if any have
>   * been copied.
>   *)
> -- 
> 2.6.2
> 
> ___
> Libguestfs mailing list
> Libguestfs@redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs


[Libguestfs] [PATCH 3/7] customize: add support for pvvxsvc

2016-04-05 Thread Cédric Bosdonnat
SUSE VMDP comes with a replacement for rhsrvany.exe named pvvxsvc.exe.
Check for either one of them instead of only rhsrvany.
---
 builder/virt-builder.pod |  13 +++-
 customize/firstboot.ml   | 169 +++
 customize/virt-customize.pod |   6 ++
 sysprep/virt-sysprep.pod |   6 ++
 v2v/virt-v2v.pod |   6 ++
 5 files changed, 117 insertions(+), 83 deletions(-)

diff --git a/builder/virt-builder.pod b/builder/virt-builder.pod
index 9a49138..be5b568 100644
--- a/builder/virt-builder.pod
+++ b/builder/virt-builder.pod
@@ -840,16 +840,17 @@ F<~root/virt-sysprep-firstboot.log>.
 =item Windows
 
 F, available from sources at
-L, is installed to run the
+L, or F, available
+with SUSE VMDP is installed to run the
 first boot scripts.  It is required, and the setup of first boot
 scripts will fail if it is not present.
 
-F is copied from the location pointed to by the
+F or F is copied from the location pointed to by the
 C environment variable; if not set, a compiled-in
 default will be used (something like F).
 
 The output of the first boot scripts is available in the guest as
-F.
+F.
 
 =back
 
@@ -1820,6 +1821,12 @@ I<--firstboot> or I<--firstboot-command> options with 
Windows guests.
 
 See also: C
 
+=item F
+
+This is a Windows binary shipped with SUSE VMDP, used to install a "firstboot"
+script in Windows guests.  It is required if you intend to use the
+I<--firstboot> or I<--firstboot-command> options with Windows guests.
+
 =back
 
 =item C
diff --git a/customize/firstboot.ml b/customize/firstboot.ml
index aa5b694..d7d791c 100644
--- a/customize/firstboot.ml
+++ b/customize/firstboot.ml
@@ -185,44 +185,52 @@ module Windows = struct
   try Sys.getenv "VIRT_TOOLS_DATA_DIR"
   with Not_found -> Guestfs_config.datadir // "virt-tools" in
 
-(* rhsrvany.exe must exist.
+(* either rhsrvany.exe or pvvxsvc.exe must exist.
  *
  * (Check also that it's not a dangling symlink but a real file).
  *)
-let rhsrvany_exe = virt_tools_data_dir // "rhsrvany.exe" in
-(try
-   let chan = open_in rhsrvany_exe in
-   close_in chan
- with
-   Sys_error msg ->
- error (f_"'%s' is missing.  This file is required in order to install 
Windows firstboot scripts.  You can get it by building rhsrvany 
(https://github.com/rwmjones/rhsrvany).  Original error: %s")
-   rhsrvany_exe msg
-);
-
-(* Create a directory for firstboot files in the guest. *)
-let firstboot_dir, firstboot_dir_win =
-  let rec loop firstboot_dir firstboot_dir_win = function
-| [] -> firstboot_dir, firstboot_dir_win
-| dir :: path ->
-  let firstboot_dir =
-if firstboot_dir = "" then "/" ^ dir else firstboot_dir // dir in
-  let firstboot_dir_win = firstboot_dir_win ^ "\\" ^ dir in
-  let firstboot_dir = g#case_sensitive_path firstboot_dir in
-  g#mkdir_p firstboot_dir;
-  loop firstboot_dir firstboot_dir_win path
-  in
-  loop "" "C:" ["Program Files"; "Red Hat"; "Firstboot"] in
-
-g#mkdir_p (firstboot_dir // "scripts");
-
-(* Copy rhsrvany to the guest. *)
-g#upload rhsrvany_exe (firstboot_dir // "rhsrvany.exe");
-
-(* Write a firstboot.bat control script which just runs the other
- * scripts in the directory.  Note we need to use CRLF line endings
- * in this script.
- *)
-let firstboot_script = sprintf "\
+let services = ["rhsrvany.exe"; "pvvxsvc.exe"] in
+let srvany = (
+  try
+List.find (
+  fun service -> (
+try
+  let chan = open_in (virt_tools_data_dir // service) in
+  close_in chan;
+  true
+with _ ->
+  false
+  )
+) services
+  with Not_found ->
+   error (f_"One of rhsrvany.exe or pvvxsvc.exe is missing in %s.  One of 
them is required in order to install Windows firstboot scripts.  You can get 
one by building rhsrvany (https://github.com/rwmjones/rhsrvany)")
+ virt_tools_data_dir
+) in (
+
+  (* Create a directory for firstboot files in the guest. *)
+  let firstboot_dir, firstboot_dir_win =
+let rec loop firstboot_dir firstboot_dir_win = function
+  | [] -> firstboot_dir, firstboot_dir_win
+  | dir :: path ->
+let firstboot_dir =
+  if firstboot_dir = "" then "/" ^ dir else firstboot_dir // dir in
+let firstboot_dir_win = firstboot_dir_win ^ "\\" ^ dir in
+let firstboot_dir = g#case_sensitive_path firstboot_dir in
+g#mkdir_p firstboot_dir;
+loop firstboot_dir firstboot_dir_win path
+in
+loop "" "C:" ["Program Files"; "Guestfs"; "Firstboot"] in
+
+

[Libguestfs] [PATCH 5/7] v2v: adapt the subkey in Enum registry to windows version

2016-04-05 Thread Cédric Bosdonnat
We need to adapt the 
Services\viostor\Enum\PCI\VEN_1AF4_1001_00021AF4_00 subkey
to what windows actually uses.
---
 v2v/windows_virtio.ml | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/v2v/windows_virtio.ml b/v2v/windows_virtio.ml
index 8a0b529..dfb7b71 100644
--- a/v2v/windows_virtio.ml
+++ b/v2v/windows_virtio.ml
@@ -139,15 +139,18 @@ and add_viostor_to_registry g inspect root current_cs 
driverdir =
   if (major == 6 && minor >= 2) || major >= 7 then (* Windows >= 8 *)
 add_viostor_to_driver_database g root arch current_cs driverdir
   else  (* Windows <= 7 *)
-add_viostor_to_critical_device_database g root current_cs
+add_viostor_to_critical_device_database g root current_cs major
 
-and add_viostor_to_critical_device_database g root current_cs =
+and add_viostor_to_critical_device_database g root current_cs major =
   (* See 
http://rwmj.wordpress.com/2010/04/30/tip-install-a-device-driver-in-a-windows-vm/
* NB: All these edits are in the HKLM\SYSTEM hive.  No other
* hive may be modified here.
*)
   let driver = "viostor.sys" in
   let driver_name = Filename.chop_extension driver in
+  (* Windows 2k3 uses '&0&', windows 2k8 '&2&' *)
+  let subkey =
+if (major == 5) then "3&13c0b0c5&0" else "3&13c0b0c5&2" in
   let regedits = [
   [ current_cs; "Control"; "CriticalDeviceDatabase"; 
"pci#ven_1af4_1001_" ],
   [ "Service", REG_SZ driver_name;
@@ -190,7 +193,7 @@ and add_viostor_to_critical_device_database g root 
current_cs =
   [ "5", REG_DWORD 0x1_l ];
 
   [ current_cs; "Services"; driver_name; "Enum" ],
-  [ "0", REG_SZ 
"PCI\\VEN_1AF4_1001_00021AF4_00\\3&13c0b0c5&0&20";
+  [ "0", REG_SZ (sprintf 
"PCI\\VEN_1AF4_1001_00021AF4_00\\%s&20" subkey);
 "Count", REG_DWORD 0x1_l;
 "NextInstance", REG_DWORD 0x1_l ];
 ] in
-- 
2.6.2

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs


[Libguestfs] [PATCH 2/7] v2v: extract controller offset discovery as a function

2016-04-05 Thread Cédric Bosdonnat
This function is needed for other drivers, move the code in order to
help sharing it later.
---
 v2v/windows_virtio.ml | 26 ++
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/v2v/windows_virtio.ml b/v2v/windows_virtio.ml
index b0d9d08..14ffc51 100644
--- a/v2v/windows_virtio.ml
+++ b/v2v/windows_virtio.ml
@@ -226,18 +226,7 @@ and add_viostor_to_driver_database g root arch current_cs 
driverdir =
*)
   let controller_path =
 [ current_cs; "Control"; "Class"; scsi_adapter_guid ] in
-  let controller_offset =
-match Windows.get_node g root controller_path with
-| None ->
-   error (f_"cannot find HKLM\\SYSTEM\\%s in the guest registry")
- (String.concat "\\" controller_path)
-| Some node ->
-   let rec loop node i =
- let controller_offset = sprintf "%04d" i in
- let child = g#hivex_node_get_child node controller_offset in
- if child = 0_L then controller_offset else loop node (i+1)
-   in
-   loop node 0 in
+  let controller_offset = get_controller_offset g root controller_path in
 
   let regedits = [
   controller_path @ [ controller_offset ],
@@ -400,6 +389,19 @@ and set_free_oem_inf g root guid driver_inf driverdir =
 g#cp (driverdir // driver_inf) ("/Windows/Inf/" ^ oem_inf);
 oem_inf
 
+and get_controller_offset g root controller_path =
+  match Windows.get_node g root controller_path with
+  | None ->
+ error (f_"cannot find HKLM\\SYSTEM\\%s in the guest registry")
+   (String.concat "\\" controller_path)
+  | Some node ->
+ let rec loop node i =
+   let controller_offset = sprintf "%04d" i in
+   let child = g#hivex_node_get_child node controller_offset in
+   if child = 0_L then controller_offset else loop node (i+1)
+ in
+ loop node 0
+
 (* Copy the matching drivers to the driverdir; return true if any have
  * been copied.
  *)
-- 
2.6.2

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs


[Libguestfs] [PATCH 6/7] v2v: quiet virtio net and balloon devices wizards

2016-04-05 Thread Cédric Bosdonnat
Setting the ConfigFlags to 0x40 for those will make windows quiet
at the first boot about those new devices. The wizard must not be
presented to the user since the needed drivers will automatically
be installed at firstboot... or worse, the wizard can even block
the installer.
---
 v2v/windows_virtio.ml | 8 
 1 file changed, 8 insertions(+)

diff --git a/v2v/windows_virtio.ml b/v2v/windows_virtio.ml
index dfb7b71..22e3e31 100644
--- a/v2v/windows_virtio.ml
+++ b/v2v/windows_virtio.ml
@@ -196,6 +196,14 @@ and add_viostor_to_critical_device_database g root 
current_cs major =
   [ "0", REG_SZ (sprintf 
"PCI\\VEN_1AF4_1001_00021AF4_00\\%s&20" subkey);
 "Count", REG_DWORD 0x1_l;
 "NextInstance", REG_DWORD 0x1_l ];
+
+  [ current_cs; "Enum"; "PCI"; "VEN_1AF4_1000_00011AF4_00"; 
subkey ^ "&18" ],
+  [ "ConfigFlags", REG_DWORD 0x40_l ];
+  [ current_cs; "Enum"; "PCI"; "VEN_1AF4_1001_00021AF4_00"; 
subkey ^ "&20" ],
+  [ "ConfigFlags", REG_DWORD 0x0_l;
+"Service", REG_SZ driver_name ];
+  [ current_cs; "Enum"; "PCI"; "VEN_1AF4_1002_00051AF4_00"; 
subkey ^ "&28" ],
+  [ "ConfigFlags", REG_DWORD 0x40_l ];
 ] in
 
   reg_import g root regedits
-- 
2.6.2

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs


[Libguestfs] [PATCH 1/7] v2v: check next free oem%d.inf in /Windows/Inf

2016-04-05 Thread Cédric Bosdonnat
It seems that checking for oem%d.inf in the DeviceIds registry entry
doesn't always list all oemXX.inf files. For example we may have
oem1.inf free in the registry key, but used in another one.

Also extract this into a separate function for later use to setup
another driver.
---
 v2v/windows_virtio.ml | 52 ++-
 1 file changed, 27 insertions(+), 25 deletions(-)

diff --git a/v2v/windows_virtio.ml b/v2v/windows_virtio.ml
index f538b36..b0d9d08 100644
--- a/v2v/windows_virtio.ml
+++ b/v2v/windows_virtio.ml
@@ -82,7 +82,7 @@ let rec install_drivers g inspect systemroot root current_cs 
rcaps =
 let target = sprintf "%s/system32/drivers/viostor.sys" systemroot in
 let target = g#case_sensitive_path target in
 g#cp source target;
-add_viostor_to_registry g inspect root current_cs;
+add_viostor_to_registry g inspect root current_cs driverdir;
 Virtio_blk
 
   | Some IDE, _ ->
@@ -133,11 +133,11 @@ let rec install_drivers g inspect systemroot root 
current_cs rcaps =
 (block, net, video)
   )
 
-and add_viostor_to_registry g inspect root current_cs =
+and add_viostor_to_registry g inspect root current_cs driverdir =
   let { i_major_version = major; i_minor_version = minor;
 i_arch = arch } = inspect in
   if (major == 6 && minor >= 2) || major >= 7 then (* Windows >= 8 *)
-add_viostor_to_driver_database g root arch current_cs
+add_viostor_to_driver_database g root arch current_cs driverdir
   else  (* Windows <= 7 *)
 add_viostor_to_critical_device_database g root current_cs
 
@@ -195,7 +195,7 @@ and add_viostor_to_critical_device_database g root 
current_cs =
 
   reg_import g root regedits
 
-and add_viostor_to_driver_database g root arch current_cs =
+and add_viostor_to_driver_database g root arch current_cs driverdir =
   (* Windows >= 8 doesn't use the CriticalDeviceDatabase.  Instead
* one must add keys into the DriverDatabase.
*)
@@ -213,27 +213,7 @@ and add_viostor_to_driver_database g root arch current_cs =
 sprintf "viostor.inf_%s_%s" arch "c86329aaeb0a7904" in
 
   let scsi_adapter_guid = "{4d36e97b-e325-11ce-bfc1-08002be10318}" in
-  (* There should be a key
-   *   HKLM\SYSTEM\DriverDatabase\DeviceIds\
-   * We want to add:
-   *   "oem1.inf"=hex(0):
-   * but if we find "oem1.inf" we'll add "oem2.inf" (etc).
-   *)
-  let oem_inf =
-let path = [ "DriverDatabase"; "DeviceIds"; scsi_adapter_guid ] in
-match Windows.get_node g root path with
-| None ->
-   error (f_"cannot find HKLM\\SYSTEM\\DriverDatabase\\DeviceIds\\%s in 
the guest registry") scsi_adapter_guid
-| Some node ->
-   let rec loop node i =
- let oem_inf = sprintf "oem%d.inf" i in
- let value = g#hivex_node_get_value node oem_inf in
- if value = 0_L then oem_inf else loop node (i+1)
-   in
-   let oem_inf = loop node 1 in
-   (* Create the key. *)
-   g#hivex_node_set_value node oem_inf (* REG_NONE *) 0_L "";
-   oem_inf in
+  let oem_inf = set_free_oem_inf g root scsi_adapter_guid "viostor.inf" 
driverdir in
 
   (* There should be a key
*   HKLM\SYSTEM\ControlSet001\Control\Class\
@@ -398,6 +378,28 @@ and add_viostor_to_driver_database g root arch current_cs =
@=hex(0012):6f,00,65,00,6d,00,31,00,2e,00,69,00,6e,00,66,00,00,00
 *)
 
+(* There should be a key
+ *   HKLM\SYSTEM\DriverDatabase\DeviceIds\
+ * We want to add:
+ *   "oem1.inf"=hex(0):
+ * but if we find "oem1.inf" we'll add "oem2.inf" (etc).
+ *)
+and set_free_oem_inf g root guid driver_inf driverdir =
+  let path = [ "DriverDatabase"; "DeviceIds"; guid ] in
+  match Windows.get_node g root path with
+  | None ->
+ error (f_"cannot find HKLM\\SYSTEM\\DriverDatabase\\DeviceIds\\%s in the 
guest registry") guid
+  | Some node ->
+let rec loop i =
+  let oem_inf = sprintf "oem%d.inf" i in
+  if not (g#exists ("/Windows/Inf/" ^ oem_inf)) then oem_inf else loop 
(i+1)
+in
+let oem_inf = loop 1 in
+(* Create the key. *)
+g#hivex_node_set_value node oem_inf (* REG_NONE *) 0_L "";
+g#cp (driverdir // driver_inf) ("/Windows/Inf/" ^ oem_inf);
+oem_inf
+
 (* Copy the matching drivers to the driverdir; return true if any have
  * been copied.
  *)
-- 
2.6.2

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs


[Libguestfs] [PATCH 0/7] Add support for SUSE virtio windows drivers

2016-04-05 Thread Cédric Bosdonnat
Hi there,

SUSE ships Virtual Machine Driver Pack for the virtio windows drivers. Get v2v
and customize to discover them and use them if available.

Cédric Bosdonnat (7):
  v2v: check next free oem%d.inf in /Windows/Inf
  v2v: extract controller offset discovery as a function
  customize: add support for pvvxsvc
  v2v: extract reusable parts of viostor regedits
  v2v: adapt the subkey in Enum registry to windows version
  v2v: quiet virtio net and balloon devices wizards
  v2v: add support for SUSE VMDP drivers

 builder/virt-builder.pod |  13 +-
 customize/firstboot.ml   | 169 ---
 customize/virt-customize.pod |   6 +
 sysprep/virt-sysprep.pod |   6 +
 v2v/convert_windows.ml   |  59 --
 v2v/virt-v2v.pod |   6 +
 v2v/windows_virtio.ml| 493 +++
 7 files changed, 476 insertions(+), 276 deletions(-)

-- 
2.6.2

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] [PATCH 7/7] v2v: add support for SUSE VMDP drivers

2016-04-05 Thread Cédric Bosdonnat
To add this support, the existing code searches for either the viostor
or the VMDP (Virtual Machine Driver Pack) files and updates the registry
accordingly.

Note that VMDP's block driver pvvxblk depends on the ballooning driver
pvvxbn.
---
 v2v/convert_windows.ml |  59 --
 v2v/windows_virtio.ml  | 113 +
 2 files changed, 141 insertions(+), 31 deletions(-)

diff --git a/v2v/convert_windows.ml b/v2v/convert_windows.ml
index 5daae6c..cb936ec 100644
--- a/v2v/convert_windows.ml
+++ b/v2v/convert_windows.ml
@@ -43,18 +43,25 @@ let convert ~keep_serial_console (g : G.guestfs) inspect 
source rcaps =
 try Sys.getenv "VIRT_TOOLS_DATA_DIR"
 with Not_found -> Guestfs_config.datadir // "virt-tools" in
 
-  (* Check if RHEV-APT exists.  This is optional. *)
-  let rhev_apt_exe = virt_tools_data_dir // "rhev-apt.exe" in
-  let rhev_apt_exe =
+  (* Check if either RHEV-APT or VMDP exists.  This is optional. *)
+  let tools = ["rhev-apt.exe"; "vmdp.exe"] in
+  let installer =
 try
-  let chan = open_in rhev_apt_exe in
-  close_in chan;
-  Some rhev_apt_exe
-with
-  Sys_error msg ->
-warning (f_"'%s' is missing.  Unable to install RHEV-APT (RHEV guest 
agent).  Original error: %s")
-  rhev_apt_exe msg;
-None in
+  let tool = List.find (
+fun item ->
+  try (
+let exe_path = virt_tools_data_dir // item in
+let chan = open_in exe_path in
+close_in chan;
+true
+  ) with _ ->
+false
+  ) tools in
+  Some (virt_tools_data_dir // tool)
+with Not_found -> (
+  warning (f_"Neither rhev-apt.exe nor vmdp.exe can be found.  Unable to 
install one of them.");
+  None
+) in
 
   (* Get the Windows %systemroot%. *)
   let systemroot = g#inspect_get_windows_systemroot inspect.i_root in
@@ -211,7 +218,14 @@ let convert ~keep_serial_console (g : G.guestfs) inspect 
source rcaps =
   (* Perform the conversion of the Windows guest. *)
 
   let rec configure_firstboot () =
-configure_rhev_apt ();
+match installer with
+| None -> info (f_"No firstboot installer to configure")
+| Some installer_path ->
+   let installer_name = Filename.basename installer_path in
+   match installer_name with
+| "rhev-apt.exe" -> configure_rhev_apt ()
+| "vmdp.exe" -> configure_vmdp ()
+| _ -> info (f_"No setup function for installer '%s'") installer_path;
 unconfigure_xenpv ();
 unconfigure_prltools ()
 
@@ -219,7 +233,7 @@ let convert ~keep_serial_console (g : G.guestfs) inspect 
source rcaps =
 (* Configure RHEV-APT (the RHEV guest agent).  However if it doesn't
  * exist just warn about it and continue.
  *)
-match rhev_apt_exe with
+match installer with
 | None -> ()
 | Some rhev_apt_exe ->
   g#upload rhev_apt_exe "/rhev-apt.exe"; (* XXX *)
@@ -236,6 +250,25 @@ net start rhev-apt
   Firstboot.add_firstboot_script g inspect.i_root
 "configure rhev-apt" fb_script
 
+  and configure_vmdp () =
+(* Configure VMDP if possible *)
+match installer with
+| None -> ()
+| Some vmdp_exe ->
+  g#upload vmdp_exe "/vmdp.exe";
+
+  let fb_script = "\
+echo V2V first boot script started
+echo Decompressing VMDP installer
+\"\\vmdp.exe\"
+cd \"VMDP-WIN*\"
+echo Installing VMDP
+setup.exe /eula_accepted /auto_reboot
+cd ..
+" in
+  Firstboot.add_firstboot_script g inspect.i_root
+"configure vmdp" fb_script
+
   and unconfigure_xenpv () =
 match xenpv_uninst with
 | None -> () (* nothing to be uninstalled *)
diff --git a/v2v/windows_virtio.ml b/v2v/windows_virtio.ml
index 22e3e31..87e39e6 100644
--- a/v2v/windows_virtio.ml
+++ b/v2v/windows_virtio.ml
@@ -62,15 +62,23 @@ let rec install_drivers g inspect systemroot root 
current_cs rcaps =
   else (
 (* Can we install the block driver? *)
 let block : guestcaps_block_type =
-  let has_viostor = g#exists (driverdir // "viostor.inf") in
+  let filenames = ["pvvxblk.sys"; "virtio_blk.sys"; "vrtioblk.sys"; 
"viostor.sys"] in
+  let driver_name = try (
+List.find (
+  fun driver_file ->
+let source = driverdir // driver_file in
+g#exists source
+) filenames
+  ) with Not_found -> "" in
+  let has_viostor = not (driver_name = "") in
   match rcaps.rcaps_block_bus, has_viostor with
   | Some Virtio_blk, false ->
-error (f_"there is no viostor (virtio block device) driver for this 
version of Windows (%d.%d %s).  virt-v2v looks for this driver in %s\n\nThe 
guest will be configured to use a slower emulated device.")
+error (f_"there is no virtio block device driver for this version of 
Windows (%d.%d %s).  virt-v2v looks for this driver in %s\n\nThe guest will be 
configured to use a slower emulated device.")
   inspect.i_major_version 

[Libguestfs] [PATCH 4/7] v2v: extract reusable parts of viostor regedits

2016-04-05 Thread Cédric Bosdonnat
There are registry entries that are needed to add some other drivers.
Extracting them into a function will help adding SUSE VMDP support.
---
 v2v/windows_virtio.ml | 311 --
 1 file changed, 176 insertions(+), 135 deletions(-)

diff --git a/v2v/windows_virtio.ml b/v2v/windows_virtio.ml
index 14ffc51..8a0b529 100644
--- a/v2v/windows_virtio.ml
+++ b/v2v/windows_virtio.ml
@@ -146,48 +146,50 @@ and add_viostor_to_critical_device_database g root 
current_cs =
* NB: All these edits are in the HKLM\SYSTEM hive.  No other
* hive may be modified here.
*)
+  let driver = "viostor.sys" in
+  let driver_name = Filename.chop_extension driver in
   let regedits = [
   [ current_cs; "Control"; "CriticalDeviceDatabase"; 
"pci#ven_1af4_1001_" ],
-  [ "Service", REG_SZ "viostor";
+  [ "Service", REG_SZ driver_name;
 "ClassGUID", REG_SZ "{4D36E97B-E325-11CE-BFC1-08002BE10318}" ];
 
   [ current_cs; "Control"; "CriticalDeviceDatabase"; 
"pci#ven_1af4_1001_0002" ],
-  [ "Service", REG_SZ "viostor";
+  [ "Service", REG_SZ driver_name;
 "ClassGUID", REG_SZ "{4D36E97B-E325-11CE-BFC1-08002BE10318}" ];
 
   [ current_cs; "Control"; "CriticalDeviceDatabase"; 
"pci#ven_1af4_1001_00021af4" ],
-  [ "Service", REG_SZ "viostor";
+  [ "Service", REG_SZ driver_name;
 "ClassGUID", REG_SZ "{4D36E97B-E325-11CE-BFC1-08002BE10318}" ];
 
   [ current_cs; "Control"; "CriticalDeviceDatabase"; 
"pci#ven_1af4_1001_00021af4_00" ],
-  [ "Service", REG_SZ "viostor";
+  [ "Service", REG_SZ driver_name;
 "ClassGUID", REG_SZ "{4D36E97B-E325-11CE-BFC1-08002BE10318}" ];
 
-  [ current_cs; "Services"; "viostor" ],
+  [ current_cs; "Services"; driver_name ],
   [ "Type", REG_DWORD 0x1_l;
 "Start", REG_DWORD 0x0_l;
 "Group", REG_SZ "SCSI miniport";
 "ErrorControl", REG_DWORD 0x1_l;
-"ImagePath", REG_EXPAND_SZ "system32\\drivers\\viostor.sys";
+"ImagePath", REG_EXPAND_SZ ("system32\\drivers\\" ^ driver);
 "Tag", REG_DWORD 0x21_l ];
 
-  [ current_cs; "Services"; "viostor"; "Parameters" ],
+  [ current_cs; "Services"; driver_name; "Parameters" ],
   [ "BusType", REG_DWORD 0x1_l ];
 
-  [ current_cs; "Services"; "viostor"; "Parameters"; "MaxTransferSize" ],
+  [ current_cs; "Services"; driver_name; "Parameters"; "MaxTransferSize" ],
   [ "ParamDesc", REG_SZ "Maximum Transfer Size";
 "type", REG_SZ "enum";
 "default", REG_SZ "0" ];
 
-  [ current_cs; "Services"; "viostor"; "Parameters"; "MaxTransferSize"; 
"enum" ],
+  [ current_cs; "Services"; driver_name; "Parameters"; "MaxTransferSize"; 
"enum" ],
   [ "0", REG_SZ "64  KB";
 "1", REG_SZ "128 KB";
 "2", REG_SZ "256 KB" ];
 
-  [ current_cs; "Services"; "viostor"; "Parameters"; "PnpInterface" ],
+  [ current_cs; "Services"; driver_name; "Parameters"; "PnpInterface" ],
   [ "5", REG_DWORD 0x1_l ];
 
-  [ current_cs; "Services"; "viostor"; "Enum" ],
+  [ current_cs; "Services"; driver_name; "Enum" ],
   [ "0", REG_SZ 
"PCI\\VEN_1AF4_1001_00021AF4_00\\3&13c0b0c5&0&20";
 "Count", REG_DWORD 0x1_l;
 "NextInstance", REG_DWORD 0x1_l ];
@@ -199,8 +201,10 @@ and add_viostor_to_driver_database g root arch current_cs 
driverdir =
   (* Windows >= 8 doesn't use the CriticalDeviceDatabase.  Instead
* one must add keys into the DriverDatabase.
*)
+  let driver = "viostor.sys" in
+  let driver_name = Filename.chop_extension driver in
 
-  let viostor_inf =
+  let inf_full =
 let arch =
   match arch with
   | "x86_64" -> "amd64"
@@ -210,145 +214,39 @@ and add_viostor_to_driver_database g root arch 
current_cs driverdir =
 (* XXX I don't know what the significance of the c863.. string is.  It
  * may even be random.
  *)
-sprintf "viostor.inf_%s_%s" arch "c86329aaeb0a7904" in
+sprintf "%s.inf_%s_%s" driver_name arch "c86329aaeb0a7904" in
 
   let scsi_adapter_guid = "{4d36e97b-e325-11ce-bfc1-08002be10318}" in
-  let oem_inf = set_free_oem_inf g root scsi_adapter_guid "viostor.inf" 
driverdir in
 
-  (* There should be a key
-   *   HKLM\SYSTEM\ControlSet001\Control\Class\
-   * There may be subkey(s) of this called "", "0001" etc.  We want
-   * to create the next free subkey.  MSFT covers the key here:
-   *   https://technet.microsoft.com/en-us/library/cc957341.aspx
-   * That page incorrectly states that the key has the form "000n".
-   * In fact we observed from real registries that the key is a
-   * decimal number that goes 0009 -> 0010 etc.
-   *)
-  let controller_path =
-[ current_cs; "Control"; "Class"; scsi_adapter_guid ] in
-  let controller_offset = get_controller_offset g root controller_path in
+  let driverdesc = "Red Hat VirtIO SCSI controller" in
+  let provider = "Red Hat, Inc." in
 
-  let regedits = [
-  controller_path @ [ controller_offset 

Re: [Libguestfs] [PATCH v2 FOR DISCUSSION ONLY 1/2] scripts: Add a script for formatting all C code in the project.

2016-04-05 Thread Richard W.M. Jones
On Tue, Apr 05, 2016 at 10:56:27AM +0100, Richard W.M. Jones wrote:
> See previous version:
> https://www.redhat.com/archives/libguestfs/2016-April/msg00021.html
> 
> The formatting in this second version isn't too bad.  Still
> a few corner cases to sort out.

I've pushed this to my private fork here:

https://github.com/rwmjones/libguestfs/tree/code-format

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs


[Libguestfs] [PATCH v2 FOR DISCUSSION ONLY 1/2] scripts: Add a script for formatting all C code in the project.

2016-04-05 Thread Richard W.M. Jones
Note this requires clang-format from clang >= 3.8.0
(3.7.0 is too old as it does not have the BreakBeforeBraces: Custom
style).
---
 .gitignore |   1 +
 scripts/code-format.pl | 193 +
 2 files changed, 194 insertions(+)
 create mode 100755 scripts/code-format.pl

diff --git a/.gitignore b/.gitignore
index ca4e89c..ad166d2 100644
--- a/.gitignore
+++ b/.gitignore
@@ -32,6 +32,7 @@ cscope.out
 Makefile
 Makefile.in
 
+/.clang-format
 /.sc-*
 /ABOUT-NLS
 /aclocal.m4
diff --git a/scripts/code-format.pl b/scripts/code-format.pl
new file mode 100755
index 000..58d3a78
--- /dev/null
+++ b/scripts/code-format.pl
@@ -0,0 +1,193 @@
+#!/usr/bin/perl -w
+# libguestfs
+# Copyright (C) 2016 Red Hat Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+# Reformat the code in libguestfs (currently only C code).
+
+use warnings;
+use strict;
+
+use YAML qw(DumpFile);
+
+# Check we are run from the top level directory.
+die "$0: you must run this script from the top level source directory\n"
+unless -f "BUGS";
+
+# Make sure we have the clang-format program.
+system ("clang-format --help >/dev/null 2>&1") == 0
+or die "$0: 'clang-format' program (from Clang) must be installed\n";
+
+# Which files to process.  Use ./scripts/code-format.pl to process all
+# files in the project, else select which files to process on the
+# command line.
+
+my @files;
+if (0 == @ARGV) {
+@files = `git ls-files '*.[ch]'`;
+chomp @files;
+} else {
+@files = @ARGV;
+}
+
+# http://clang.llvm.org/docs/ClangFormatStyleOptions.html
+my $clang_stylesheet = {
+Language => "Cpp",
+AccessModifierOffset => -2,
+AlignAfterOpenBracket => "true",
+AlignConsecutiveAssignments => "false",
+AlignEscapedNewlinesLeft => "true",
+AlignOperands => "true",
+AlignTrailingComments => "true",
+AllowAllParametersOfDeclarationOnNextLine => "true",
+AllowShortBlocksOnASingleLine => "false",
+AllowShortCaseLabelsOnASingleLine => "false",
+AllowShortFunctionsOnASingleLine => "All",
+AllowShortIfStatementsOnASingleLine => "false",
+AllowShortLoopsOnASingleLine => "false",
+AlwaysBreakAfterDefinitionReturnType => "TopLevel",
+AlwaysBreakBeforeMultilineStrings => "false",
+AlwaysBreakTemplateDeclarations => "false",
+BinPackArguments => "true",
+BinPackParameters => "true",
+BraceWrapping => {
+AfterClass => "false",
+AfterControlStatement => "false",
+AfterEnum => "false",
+AfterFunction => "true",
+AfterNamespace => "true",
+AfterObjCDeclaration => "false",
+AfterStruct => "false",
+AfterUnion => "true",
+BeforeCatch => "false",
+BeforeElse => "true",
+#AfterIndentBraces => "false",  -- only in clang > 3.8.0
+},
+BreakBeforeBinaryOperators => "None",
+BreakBeforeBraces => "Custom",
+BreakBeforeTernaryOperators => "true",
+BreakConstructorInitializersBeforeComma => "false",
+#BreakStringLiterals => "false",  -- only in clang > 3.8.0
+ColumnLimit => 76,
+CommentPragmas => "51 Franklin Street",
+ConstructorInitializerAllOnOneLineOrOnePerLine => "false",
+ConstructorInitializerIndentWidth => 2,
+ContinuationIndentWidth => 2,
+Cpp11BracedListStyle => "false",
+DerivePointerAlignment => "false",
+DisableFormat => "false",
+ExperimentalAutoDetectBinPacking => "false",
+ForEachMacros => [],
+IndentCaseLabels => "false",
+IndentWidth => 2,
+IndentWrappedFunctionNames => "false",
+KeepEmptyLinesAtTheStartOfBlocks => "true",
+MacroBlockBegin => "",
+MacroBlockEnd => "",
+MaxEmptyLinesToKeep => 1,
+NamespaceIndentation => "None",
+ObjCBlockIndentWidth => 2,
+ObjCSpaceAfterProperty => "false",
+ObjCSpaceBeforeProtocolList => "true",
+PenaltyBreakBeforeFirstCallParameter => 19,
+PenaltyBreakComment => 300,
+PenaltyBreakFirstLessLess => 120,
+PenaltyBreakString => 1000,
+PenaltyExcessCharacter => 100,
+PenaltyReturnTypeOnItsOwnLine => 60,
+PointerAlignment => "Right",
+SortIncludes => "false",
+SpaceAfterCStyleCast => "false",
+SpaceBeforeAssignmentOperators => "true",
+SpaceBeforeParens => "Always",
+SpaceInEmptyParentheses => 

[Libguestfs] [PATCH v2 FOR DISCUSSION ONLY 1/2] scripts: Add a script for formatting all C code in the project.

2016-04-05 Thread Richard W.M. Jones
See previous version:
https://www.redhat.com/archives/libguestfs/2016-April/msg00021.html

The formatting in this second version isn't too bad.  Still
a few corner cases to sort out.

Rich.

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs


Re: [Libguestfs] [PATCH v2 3/5] daemon: Added internal_filesystem_walk command

2016-04-05 Thread Pino Toscano
On Monday 04 April 2016 23:07:05 noxdafox wrote:
> On 04/04/16 15:15, Pino Toscano wrote:
> > On Monday 04 April 2016 14:58:35 NoxDaFox wrote:
>  +
>  +static int open_filesystem (const char *device,
>  +TSK_IMG_INFO **img, TSK_FS_INFO **fs);
>  +static TSK_WALK_RET_ENUM fswalk_callback (TSK_FS_FILE *fsfile,
>  +  const char *path, void *data);
> >>> Single line for forward declarations.
> >>>
> >> Even if they are longer than 80 chars?
> > Yep.
> >
>  +static char file_type (TSK_FS_FILE *fsfile);
>  +static int send_dirent_info (guestfs_int_tsk_dirent *dirent);
>  +static void reply_with_tsk_error (const char *funcname);
>  +
>  +int
>  +do_internal_filesystem_walk (const mountable_t *mountable)
>  +{
>  +  int ret = -1;
>  +  TSK_FS_INFO *fs = NULL;
>  +  TSK_IMG_INFO *img = NULL;  /* Used internally by tsk_fs_dir_walk */
>  +  int flags = TSK_FS_DIR_WALK_FLAG_ALLOC | TSK_FS_DIR_WALK_FLAG_UNALLOC
> >>> |
>  +TSK_FS_DIR_WALK_FLAG_RECURSE | TSK_FS_DIR_WALK_FLAG_NOORPHAN;
>  +
>  +  ret = open_filesystem (mountable->device, , );
>  +  if (ret < 0)
>  +return ret;
>  +
>  +  reply (NULL, NULL);  /* Reply message. */
>  +
>  +  ret = tsk_fs_dir_walk (fs, fs->root_inum, flags, fswalk_callback,
> >>> NULL);
>  +  if (ret == 0)
>  +ret = send_file_end (0);  /* File transfer end. */
>  +  else
>  +send_file_end (1);  /* Cancel file transfer. */
>  +
>  +  fs->close (fs);
>  +  img->close (img);
>  +
>  +  return ret;
>  +}
>  +
>  +/* Inspect the device and initialises the img and fs structures.
>  + * Return 0 on success, -1 on error.
>  + */
>  +static int
>  +open_filesystem (const char *device, TSK_IMG_INFO **img, TSK_FS_INFO
> >>> **fs)
>  +{
>  +  const char *images[] = { device };
>  +
>  +  *img = tsk_img_open (1, images, TSK_IMG_TYPE_DETECT , 0);
>  +  if (*img == NULL) {
>  +reply_with_tsk_error ("tsk_image_open");
>  +return -1;
>  +  }
>  +
>  +  *fs = tsk_fs_open_img (*img, 0, TSK_FS_TYPE_DETECT);
>  +  if (*fs == NULL) {
>  +reply_with_tsk_error ("tsk_fs_open_img");
>  +(*img)->close (*img);
>  +return -1;
>  +  }
>  +
>  +  return 0;
>  +}
>  +
>  +/* Filesystem walk callback, it gets called on every FS node.
>  + * Parse the node, encode it into an XDR structure and send it to the
> >>> appliance.
>  + * Return TSK_WALK_CONT on success, TSK_WALK_ERROR on error.
>  + */
>  +static TSK_WALK_RET_ENUM
>  +fswalk_callback (TSK_FS_FILE *fsfile, const char *path, void *data)
>  +{
>  +  int ret = 0;
>  +  CLEANUP_FREE char *fname = NULL;
>  +  struct guestfs_int_tsk_dirent dirent;
>  +
>  +  /* Ignore ./ and ../ */
>  +  ret = TSK_FS_ISDOT (fsfile->name->name);
>  +  if (ret != 0)
>  +return TSK_WALK_CONT;
>  +
>  +  /* Build the full relative path of the entry */
>  +  ret = asprintf_nowarn (, "%Q%Q", path, fsfile->name->name);
> >>> Why the quoting?  We don't quote results in similar APIs (e.g. readdir).
> >>>
> >> I didn't understand this one. I checked daemon/readdir.c and I found no
> >> asprintf examples there.
> > $ ./run guestfish -N fs -m /dev/sda1 touch "/file with spaces" : readdir /
> > [0] = {
> >ino: 12
> >ftyp: r
> >name: file with spaces
> > }
> > [1] = {
> >ino: 2
> >ftyp: d
> >name: .
> > }
> > [2] = {
> >ino: 11
> >ftyp: d
> >name: lost+found
> > }
> > [3] = {
> >ino: 2
> >ftyp: d
> >name: ..
> > }
> >
> > You can see the file names are not quoted.
> 
> As long as I'm not missing something, the double quote there is not 
> adding quotes to the names but passing the string formatting parameter 
> to 'asprintf'.
> int asprintf(char **strp, const char *fmt, ...);
> 
> If I remove it, I get this error.
> 
> tsk.c: In function 'fswalk_callback':
> tsk.c:112:34: error: expected expression before '%' token
> ret = asprintf_nowarn (, %Q%Q, path, fsfile->name->name);
> 
> If I keep it, I get this output.
> 
> ./run guestfish --ro -a ubuntu.qcow2 run : filesystem_walk /dev/sda1 | less
> [0] = {
>tsk_inode: 11
>tsk_type: d
>tsk_size: 16384
>tsk_name: lost\+found
>tsk_allocated: 1
> }
> [1] = {
>tsk_inode: 12
>tsk_type: l
>tsk_size: 33
>tsk_name: initrd.img
>tsk_allocated: 1
> }
> 
> As you can see names are not quoted.

Sorry, I said "quoted" because of %Q -- they are escaped.  Just join
path and filename without escaping them.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs