Re: [Libguestfs] [PATCH 1/2] inspect: fstab: Canonicalize paths appearing in fstab.

2016-12-07 Thread Pino Toscano 
On Tuesday, 6 December 2016 09:46:25 CET Richard W.M. Jones wrote:
> For example, converts "///usr//local//" -> "/usr/local".
> ---
>  src/inspect-fs-unix.c | 52 
> +++
>  1 file changed, 52 insertions(+)
> 
> diff --git a/src/inspect-fs-unix.c b/src/inspect-fs-unix.c
> index a1a757c..0fea9c8 100644
> --- a/src/inspect-fs-unix.c
> +++ b/src/inspect-fs-unix.c
> @@ -89,6 +89,7 @@ static char *resolve_fstab_device (guestfs_h *g, const char 
> *spec,
> Hash_table *md_map,
> enum inspect_os_type os_type);
>  static int inspect_with_augeas (guestfs_h *g, struct inspect_fs *fs, const 
> char **configfiles, int (*f) (guestfs_h *, struct inspect_fs *));
> +static void canonical_mountpoint (char *mp);
>  
>  /* Hash structure for uuid->path lookups */
>  typedef struct md_uuid {
> @@ -1286,6 +1287,9 @@ check_fstab (guestfs_h *g, struct inspect_fs *fs)
>  if (mp == NULL)
>return -1;
>  
> +/* Canonicalize the path, so "///usr//local//" -> "/usr/local" */
> +canonical_mountpoint (mp);
> +
>  /* Ignore certain mountpoints. */
>  if (STRPREFIX (mp, "/dev/") ||
>  STREQ (mp, "/dev") ||
> @@ -2081,3 +2085,51 @@ make_augeas_path_expression (guestfs_h *g, const char 
> **configfiles)
>debug (g, "augeas pathexpr = %s", ret);
>return ret;
>  }
> +
> +/* Canonicalize the path, so "///usr//local//" -> "/usr/local"
> + *
> + * The path is modified in place because the result is always
> + * the same length or shorter than the argument passed.
> + */
> +static void
> +drop_char (char *mp)
> +{
> +  size_t len = strlen (mp);
> +  memmove ([0], [1], len);
> +}
> +
> +static void
> +canonical_mountpoint_recursive (char *mp)
> +{
> +  if (mp[0] == '\0')
> +return;
> +
> +  /* Remove trailing slashes. */
> +  if (mp[0] == '/' && mp[1] == '\0') {
> +mp[0] = '\0';
> +return;
> +  }
> +
> +  /* Replace multiple slashes with single slashes. */
> +  if (mp[0] == '/' && mp[1] == '/') {
> +drop_char (mp);
> +canonical_mountpoint_recursive (mp);
> +return;
> +  }
> +
> +  canonical_mountpoint_recursive ([1]);
> +}
> +
> +static void
> +canonical_mountpoint (char *mp)
> +{
> +  /* Collapse multiple leading slashes into a single slash ... */
> +  while (mp[0] == '/' && mp[1] == '/')
> +drop_char (mp);
> +
> +  /* ... and then continue, skipping the leading slash. */
> +  if (mp[0] == '/')
> +canonical_mountpoint_recursive ([1]);
> +  else
> +canonical_mountpoint_recursive (mp);
> +}

This implementation looks a bit inefficient to me (recursive, moving
bytes and using strlen for every char removed, etc). What about the
following implementation?

  void canonicalize(char *s)
  {
size_t len = strlen(s);
char *orig = s;

s = strchr(s, '/');
while (s != NULL && *s != 0) {
  char *pos = s + 1;
  char *p = pos;
  while (*p == '/')
++p;
  if (p - pos > 0) {
memmove(pos, p, len - (p - orig) + 1);
len -= p - pos;
  }

  s = strchr(pos, '/');
}
orig[len] = 0;
  }

The only behaviour change with the committed implementation is that it
does not remove trailing slashes, but IMHO they could be left there
(or removed after the canonicalization function, just once).

-- 
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 0/5] Improve inspection of /usr filesystems

2016-12-07 Thread Richard W.M. Jones 
On Tue, Dec 06, 2016 at 04:29:17PM +0100, Pino Toscano wrote:
> Hi,
> 
> this patch series improves the way /usr filesystems are handled: tag
> them appropriately, so later on we can find them and merge results they
> contain directly back for the root filesystem.
> 
> The series includes also a new private debug API, and its usage to fix
> the resolution of /dev/mapper/.. devices found in fstab; without it,
> LVM /usr filesystems are not recognized as belonging to their roots.
> Maybe a better API for this could be added, but since it's something
> only related to the appliance then can stay internal for now. (Better
> suggestions always welcome, of course.)

Firstly, I still think we need:

  [PATCH 1/2] inspect: fstab: Canonicalize paths appearing in fstab.

although it's not related to the current patch series.

In regards to this series:

> Pino Toscano (5):
>   inspect: change is_root flag into enum
>   inspect: mark CoreOS /usr partitions with own USR role

These are fine.

>   daemon: debug: new "exists" subcommand
>   inspect: fix existance check of /dev/mapper devices

This is very ugly.  We really shouldn't be calling a debug API from
inspection code.

>   inspect: gather info from /usr filesystems as well (RHBZ#1401474)

I have specific comments on this patch.

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 4/5] inspect: fix existance check of /dev/mapper devices

2016-12-07 Thread Richard W.M. Jones 
On Tue, Dec 06, 2016 at 04:29:21PM +0100, Pino Toscano wrote:
> @@ -1820,7 +1833,7 @@ resolve_fstab_device (guestfs_h *g, const char *spec, 
> Hash_table *md_map,
>char *type, *slice, *disk, *part;
>int r;
>  
> -  if (STRPREFIX (spec, "/dev/mapper/") && guestfs_exists (g, spec) > 0) {
> +  if (STRPREFIX (spec, "/dev/mapper/") && dev_mapper_exists (g, spec) > 0) {
...
device = guestfs_lvm_canonical_lv_name (g, spec);
  }


I think really the problem is with the call to
guestfs_lvm_canonical_lv_name.

Can we not replace this code with:

if (STRPREFIX (spec, "/dev/mapper/")) {
...
  guestfs_push_error_handler (g, NULL, NULL);
  device = guestfs_lvm_canonical_lv_name (g, spec);
  guestfs_pop_error_handler (g);
  if (device == NULL) {
if (guestfs_last_errno (g) == ENOENT)
  // ignore error
else {
  guestfs_int_error_errno (/* copy the handle error message & errno here 
*/);
  return -1;
  }
}

?

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] v2v: Rename RHEV to RHV throughout.

2016-12-07 Thread Richard W.M. Jones 
On Tue, Dec 06, 2016 at 04:41:52PM +0100, Pino Toscano wrote:
> On Thursday, 1 December 2016 14:35:07 CET Richard W.M. Jones wrote:
> > You can now use -o rhv (-o rhev is supported for compatibility).
> 
> This LGTM -- the only concern is that "output:rhev" will disappear from
> the machine-readable output (and thus potentially breaking users).
> I have PoC for handling better aliases for input & output modules,
> I will polish and submit it.

Hopefully they are parsing the --machine-readable output and so this
won't be a problem :-)  virt-p2v should be able to handle it anyhow.

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 0/5] Improve inspection of /usr filesystems

2016-12-07 Thread Pino Toscano 
On Wednesday, 7 December 2016 08:29:54 CET Richard W.M. Jones wrote:
> On Tue, Dec 06, 2016 at 04:29:17PM +0100, Pino Toscano wrote:
> > Hi,
> > 
> > this patch series improves the way /usr filesystems are handled: tag
> > them appropriately, so later on we can find them and merge results they
> > contain directly back for the root filesystem.
> > 
> > The series includes also a new private debug API, and its usage to fix
> > the resolution of /dev/mapper/.. devices found in fstab; without it,
> > LVM /usr filesystems are not recognized as belonging to their roots.
> > Maybe a better API for this could be added, but since it's something
> > only related to the appliance then can stay internal for now. (Better
> > suggestions always welcome, of course.)
> 
> Firstly, I still think we need:
> 
>   [PATCH 1/2] inspect: fstab: Canonicalize paths appearing in fstab.
> 
> although it's not related to the current patch series.

This was not an issue in the data of RHBZ#140147, do you have an example
of guest with such paths in fstab? So far I never seen one myself.

> In regards to this series:
> 
> > Pino Toscano (5):
> >   inspect: change is_root flag into enum
> >   inspect: mark CoreOS /usr partitions with own USR role
> 
> These are fine.

Pushed, thanks.

> >   daemon: debug: new "exists" subcommand
> >   inspect: fix existance check of /dev/mapper devices
> 
> This is very ugly.  We really shouldn't be calling a debug API from
> inspection code.

As mentioned above, any better suggestion is welcome. I didn't want to
add a new public API for inspecting files in the appliance itself --
what about something like internal_exists?

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] v2v: Rename RHEV to RHV throughout.

2016-12-07 Thread Pino Toscano 
On Wednesday, 7 December 2016 08:34:35 CET Richard W.M. Jones wrote:
> On Tue, Dec 06, 2016 at 04:41:52PM +0100, Pino Toscano wrote:
> > On Thursday, 1 December 2016 14:35:07 CET Richard W.M. Jones wrote:
> > > You can now use -o rhv (-o rhev is supported for compatibility).
> > 
> > This LGTM -- the only concern is that "output:rhev" will disappear from
> > the machine-readable output (and thus potentially breaking users).
> > I have PoC for handling better aliases for input & output modules,
> > I will polish and submit it.
> 
> Hopefully they are parsing the --machine-readable output and so this
> won't be a problem :-)

Well exactly: newer virt-v2v with "older" ovirt/rhv will not see
output:rhev anymore and thus not enable the VM import from VMware/etc.

-- 
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 v2] v2v: Rename RHEV to RHV throughout.

2016-12-07 Thread Richard W.M. Jones 
v2:

 - Fix virt-p2v messages too.

Rich.

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

Re: [Libguestfs] [PATCH] v2v: Rename RHEV to RHV throughout.

2016-12-07 Thread Pino Toscano 
On Wednesday, 7 December 2016 10:10:36 CET Richard W.M. Jones wrote:
> On Wed, Dec 07, 2016 at 10:56:05AM +0100, Pino Toscano wrote:
> > On Wednesday, 7 December 2016 08:34:35 CET Richard W.M. Jones wrote:
> > > On Tue, Dec 06, 2016 at 04:41:52PM +0100, Pino Toscano wrote:
> > > > On Thursday, 1 December 2016 14:35:07 CET Richard W.M. Jones wrote:
> > > > > You can now use -o rhv (-o rhev is supported for compatibility).
> > > > 
> > > > This LGTM -- the only concern is that "output:rhev" will disappear from
> > > > the machine-readable output (and thus potentially breaking users).
> > > > I have PoC for handling better aliases for input & output modules,
> > > > I will polish and submit it.
> > > 
> > > Hopefully they are parsing the --machine-readable output and so this
> > > won't be a problem :-)
> > 
> > Well exactly: newer virt-v2v with "older" ovirt/rhv will not see
> > output:rhev anymore and thus not enable the VM import from VMware/etc.
> 
> I think you mean newer virt-p2v and older virt-v2v?

No, I mean other users of v2v which looks at the machine-readable
output. Not sure whether ovirt/rhv is one of them, but others might.

-- 
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] libguestfs and exporting to OVA/OVF

2016-12-07 Thread Emmanuel Kasper 
Am 22. Oktober 2016 00:03:30 MESZ, schrieb Emmanuel Kasper :
>Le 21/10/2016 à 16:08, Richard W.M. Jones a écrit :
>> On Thu, Oct 20, 2016 at 10:43:26PM +0200, Emmanuel Kasper wrote:
>>> Hi
>>> I've been looking for a standalone tool to create OVA/OVF VM files
>based
>>> on a disk image and found none. So I was thinking to write my own.
>>> Would you be interested in having such a tool in the libguestfs
>umbrella ?
>> 
>> There is this:
>> 
>>   http://git.annexia.org/?p=import-to-ovirt.git
>> 
>> which can generate OVF, and with not very much extra work could
>> generate OVAs.
>> 
>> The problem is that OVF is not a reliable standard.  Sure, there is a
>> standards organization behind it, but there is in practice no
>interop.
>> You have to know the target hypervisor in order to be able to create
>> OVF which will work, and the OVF is quite different for each target.
>
>
>Thanks for the hint for import-to-ovirt, the code is easy to read and
>follow, and is close to what I want to achieve.
>
>A good part of the script is ovirt specific but I am going to reuse the
>part where you generate the OVF file.
>
>I'll send you a link when I have something to show.
>
>___
>Libguestfs mailing list
>Libguestfs@redhat.com
>https://www.redhat.com/mailman/listinfo/libguestfs

So I adapted on my spare time import-to-ovirt and I have now my  import2vbox.
I had to adapt the generated ovf and disable the rh(e)v specific stuff but 
nothing extraordinary complicated.
Code is here:

https://github.com/EmmanuelKasper/import2vbox

Thanks for the initial pointer to import-to-ovirt

Emmanuel


-- 
Diese Nachricht wurde von meinem Android-Mobiltelefon mit K-9 Mail gesendet.___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH 5/5] inspect: gather info from /usr filesystems as well (RHBZ#1401474)

2016-12-07 Thread Richard W.M. Jones 
On Tue, Dec 06, 2016 at 04:29:22PM +0100, Pino Toscano wrote:
> +static void
> +collect_linux_inspection_info_for (guestfs_h *g, struct inspect_fs *root)
> +{
> +  size_t i;
> +  struct inspect_fs *usr = NULL;
> +
> +  for (i = 0; i < g->nr_fses; ++i) {
> +struct inspect_fs *fs = >fses[i];
> +size_t j;
> +
> +if (!(fs->distro == root->distro || fs->distro == OS_DISTRO_UNKNOWN) ||
> +fs->role != OS_ROLE_USR)
> +  continue;
> +
> +for (j = 0; j < root->nr_fstab; ++j) {
> +  if (STREQ (fs->mountable, root->fstab[j].mountable)) {
> +usr = fs;
> +goto got_usr;
> +  }
> +}
> +  }
> +
> +  if (usr == NULL)

I think this should be:

  assert (usr == NULL);

unless there's some way to reach this point and usr != NULL.

> +return;
> +
> + got_usr:
> +  /* If the version information in /usr is not null, then most probably
> +   * there was an os-release file there, so reset what is in root
> +   * and pick the results from /usr.
> +   */
> +  if (!version_is_null (>version)) {
> +root->distro = OS_DISTRO_UNKNOWN;
> +free (root->product_name);
> +root->product_name = NULL;
> +  }
> +
> +  guestfs_int_merge_fs_inspections (g, root, usr);
> +}

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] v2v: Rename RHEV to RHV throughout.

2016-12-07 Thread Richard W.M. Jones 
You can now use -o rhv (-o rhev is supported for compatibility).
---
 p2v/gui.c |   5 +-
 p2v/virt-p2v.pod  |  13 ++-
 v2v/Makefile.am   |   8 +-
 v2v/OVF.ml|  30 +++
 v2v/OVF.mli   |   2 +-
 v2v/changeuid.mli |   4 +-
 v2v/cmdline.ml|  12 +--
 v2v/convert_windows.ml|   2 +-
 v2v/{output_rhev.ml => output_rhv.ml} |  26 +++---
 v2v/{output_rhev.mli => output_rhv.mli}   |   8 +-
 v2v/output_vdsm.ml|   4 +-
 v2v/test-harness/virt-v2v-test-harness.pod|   4 +-
 v2v/test-v2v-no-copy.sh   |   6 +-
 v2v/{test-v2v-o-rhev.sh => test-v2v-o-rhv.sh} |  10 +--
 v2v/types.mli |   2 +-
 v2v/virt-v2v.pod  | 112 +-
 16 files changed, 126 insertions(+), 122 deletions(-)
 rename v2v/{output_rhev.ml => output_rhv.ml} (90%)
 rename v2v/{output_rhev.mli => output_rhv.mli} (83%)
 rename v2v/{test-v2v-o-rhev.sh => test-v2v-o-rhv.sh} (95%)

diff --git a/p2v/gui.c b/p2v/gui.c
index dee8b83..7547e45 100644
--- a/p2v/gui.c
+++ b/p2v/gui.c
@@ -855,7 +855,7 @@ create_conversion_dialog (struct config *config)
   set_alignment (o_label, 1., 0.5);
   o_combo = gtk_combo_box_text_new ();
   gtk_label_set_mnemonic_widget (GTK_LABEL (o_label), o_combo);
-  gtk_widget_set_tooltip_markup (o_combo, _("libvirt means send the 
converted guest to libvirt-managed KVM on the conversion server.  local 
means put it in a directory on the conversion server.  rhev means write 
it to RHEV-M/oVirt.  glance means write it to OpenStack Glance.  See the 
virt-v2v(1) manual page for more information about output options."));
+  gtk_widget_set_tooltip_markup (o_combo, _("libvirt means send the 
converted guest to libvirt-managed KVM on the conversion server.  local 
means put it in a directory on the conversion server.  rhv means write 
it to RHV-M/oVirt.  glance means write it to OpenStack Glance.  See the 
virt-v2v(1) manual page for more information about output options."));
   repopulate_output_combo (config);
   table_attach (output_tbl, o_combo,
 1, 2, 0, 1, GTK_FILL, GTK_FILL, 1, 1);
@@ -878,7 +878,7 @@ create_conversion_dialog (struct config *config)
   set_alignment (os_label, 1., 0.5);
   os_entry = gtk_entry_new ();
   gtk_label_set_mnemonic_widget (GTK_LABEL (os_label), os_entry);
-  gtk_widget_set_tooltip_markup (os_entry, _("For local, put the 
directory name on the conversion server.  For rhev, put the Export 
Storage Domain (server:/mountpoint).  For others, leave this field blank."));
+  gtk_widget_set_tooltip_markup (os_entry, _("For local, put the 
directory name on the conversion server.  For rhv, put the Export 
Storage Domain (server:/mountpoint).  For others, leave this field blank."));
   if (config->output_storage != NULL)
 gtk_entry_set_text (GTK_ENTRY (os_entry), config->output_storage);
   table_attach (output_tbl, os_entry,
@@ -1077,6 +1077,7 @@ repopulate_output_combo (struct config *config)
   if (output_drivers == NULL) {
 gtk_combo_box_text_append_text (GTK_COMBO_BOX_TEXT (o_combo), "libvirt");
 gtk_combo_box_text_append_text (GTK_COMBO_BOX_TEXT (o_combo), "local");
+/* Use rhev instead of rhv here so we can work with old virt-v2v. */
 gtk_combo_box_text_append_text (GTK_COMBO_BOX_TEXT (o_combo), "rhev");
 if (output == NULL || STREQ (output, "libvirt"))
   gtk_combo_box_set_active (GTK_COMBO_BOX (o_combo), 0);
diff --git a/p2v/virt-p2v.pod b/p2v/virt-p2v.pod
index 2606983..c75be2b 100644
--- a/p2v/virt-p2v.pod
+++ b/p2v/virt-p2v.pod
@@ -11,9 +11,8 @@ virt-p2v - Convert a physical machine to use KVM
 =head1 DESCRIPTION
 
 Virt-p2v converts a physical machine to run virtualized on KVM,
-managed by libvirt, OpenStack, oVirt, Red Hat Enterprise
-Virtualisation (RHEV), or one of the other targets supported by
-L.
+managed by libvirt, OpenStack, oVirt, Red Hat Virtualisation (RHV), or
+one of the other targets supported by L.
 
 Normally you don't run the virt-p2v program directly.  Instead you
 have to boot the physical machine using the bootable CD-ROM, ISO or
@@ -394,13 +393,13 @@ interfaces to the target C network.
 You give a comma-separated list of C pairs, plus
 optionally a default target.  For example:
 
- p2v.network=em1:rhevm
+ p2v.network=em1:rhvm
 
-maps interface C to target network C.
+maps interface C to target network C.
 
- p2v.network=em1:rhevm,em2:management,other
+ p2v.network=em1:rhvm,em2:management,other
 
-maps interface C to C, and C to C, and
+maps interface C to C, and C to C, and
 any other interface that is found to C.
 
 =item

Re: [Libguestfs] [PATCH] v2v: Rename RHEV to RHV throughout.

2016-12-07 Thread Richard W.M. Jones 
On Wed, Dec 07, 2016 at 10:56:05AM +0100, Pino Toscano wrote:
> On Wednesday, 7 December 2016 08:34:35 CET Richard W.M. Jones wrote:
> > On Tue, Dec 06, 2016 at 04:41:52PM +0100, Pino Toscano wrote:
> > > On Thursday, 1 December 2016 14:35:07 CET Richard W.M. Jones wrote:
> > > > You can now use -o rhv (-o rhev is supported for compatibility).
> > > 
> > > This LGTM -- the only concern is that "output:rhev" will disappear from
> > > the machine-readable output (and thus potentially breaking users).
> > > I have PoC for handling better aliases for input & output modules,
> > > I will polish and submit it.
> > 
> > Hopefully they are parsing the --machine-readable output and so this
> > won't be a problem :-)
> 
> Well exactly: newer virt-v2v with "older" ovirt/rhv will not see
> output:rhev anymore and thus not enable the VM import from VMware/etc.

I think you mean newer virt-p2v and older virt-v2v?  It should
populate the combo box with the output of --machine-readable, so it
should show the (old) 'rhev' option in this case.

For older virt-p2v, newer virt-v2v, '-o rhev' still works.

However I noticed there are places in virt-p2v where messages
refer to "rhev", so I'll fix those in a follow-up patch.

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

Re: [Libguestfs] [PATCH v2] v2v: Rename RHEV to RHV throughout.

2016-12-07 Thread Pino Toscano 
On Wednesday, 7 December 2016 10:16:18 CET Richard W.M. Jones wrote:
> You can now use -o rhv (-o rhev is supported for compatibility).
> ---

I didn't check the v2v parts (I assume they are the same as in v1 of
the patch) -- the new p2v bits LGTM.

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] v2v: -o vdsm: Add --vdsm-compat flag. -o rhev: Drop support for RHV < 4.1 (RHBZ#1400205).

2016-12-07 Thread Richard W.M. Jones 
Support for RHEV with RHEL 6 nodes required us to output the old style
qcow2 compat=0.10 images.  Since RHEV 3.6 GA, RHEL 6 has not been
supported as a RHEV node type.  Since RHV 4.1, compat=1.1 is
supported.  (Support for compat=1.1 is uncertain in RHV 4.0 even on
RHEL 7 nodes.)

There are significant downsides to using qcow2 compat=0.10 instead of
the modern default (compat=1.1).

Therefore this patch does two things:

For -o rhev, it drops support for compat=0.10 completely.  You must
use RHV 4.1.

For -o vdsm, it adds an interim flag (--vdsm-compat=0.10 or
--vdsm-compat=1.1) which controls the compat level of the qcow2 output
file.  VDSM should use --vdsm-compat=1.1 when it is known that modern
qemu is available.  We can make this the default later when all RHV
instances have moved to 4.1.

It also adds:

  vdsm-compat-option

to the `virt-v2v --machine-readable' output to indicate that this flag
can be used.

Thanks: Yaniv Kaul, Michal Skrivanek.
---
 v2v/cmdline.ml |  7 +++
 v2v/output_rhev.ml |  4 
 v2v/output_vdsm.ml | 11 ---
 v2v/output_vdsm.mli|  1 +
 v2v/test-v2v-o-vdsm-options.sh | 11 +--
 v2v/virt-v2v.pod   | 21 -
 6 files changed, 45 insertions(+), 10 deletions(-)

diff --git a/v2v/cmdline.ml b/v2v/cmdline.ml
index 2d0a10a..1d325af 100644
--- a/v2v/cmdline.ml
+++ b/v2v/cmdline.ml
@@ -66,6 +66,9 @@ let parse_cmdline () =
   let vdsm_vm_uuid = ref None in
   let vdsm_ovf_output = ref None in (* default "." *)
 
+  let vdsm_compat = ref "0.10" in
+  let set_vdsm_compat s = vdsm_compat := s in
+
   let set_string_option_once optname optref arg =
 match !optref with
 | Some _ ->
@@ -198,6 +201,7 @@ let parse_cmdline () =
 [ L"print-source" ], Getopt.Set print_source, s_"Print source and stop";
 [ L"qemu-boot" ], Getopt.Set qemu_boot,   s_"Boot in qemu (-o qemu 
only)";
 [ L"root" ],Getopt.String ("ask|... ", set_root_choice), s_"How to 
choose root filesystem";
+[ L"vdsm-compat" ], Getopt.Symbol ("0.10|1.1", ["0.10"; "1.1"], 
set_vdsm_compat), s_"Write qcow2 with compat=0.10|1.1";
 [ L"vdsm-image-uuid" ], Getopt.String ("uuid", add_vdsm_image_uuid), 
s_"Output image UUID(s)";
 [ L"vdsm-vol-uuid" ], Getopt.String ("uuid", add_vdsm_vol_uuid), s_"Output 
vol UUID(s)";
 [ L"vdsm-vm-uuid" ], Getopt.String ("uuid", set_string_option_once 
"--vdsm-vm-uuid" vdsm_vm_uuid),
@@ -259,6 +263,7 @@ read the man page virt-v2v(1).
   let print_source = !print_source in
   let qemu_boot = !qemu_boot in
   let root_choice = !root_choice in
+  let vdsm_compat = !vdsm_compat in
   let vdsm_image_uuids = List.rev !vdsm_image_uuids in
   let vdsm_vol_uuids = List.rev !vdsm_vol_uuids in
   let vdsm_vm_uuid = !vdsm_vm_uuid in
@@ -272,6 +277,7 @@ read the man page virt-v2v(1).
 printf "virt-v2v\n";
 printf "libguestfs-rewrite\n";
 printf "colours-option\n";
+printf "vdsm-compat-option\n";
 List.iter (printf "input:%s\n") (Modules_list.input_modules ());
 List.iter (printf "output:%s\n") (Modules_list.output_modules ());
 List.iter (printf "convert:%s\n") (Modules_list.convert_modules ());
@@ -415,6 +421,7 @@ read the man page virt-v2v(1).
 vol_uuids = vdsm_vol_uuids;
 vm_uuid = vdsm_vm_uuid;
 ovf_output = vdsm_ovf_output;
+compat = vdsm_compat;
   } in
   Output_vdsm.output_vdsm os vdsm_params output_alloc in
 
diff --git a/v2v/output_rhev.ml b/v2v/output_rhev.ml
index e45043b..3280150 100644
--- a/v2v/output_rhev.ml
+++ b/v2v/output_rhev.ml
@@ -248,10 +248,6 @@ object
 Changeuid.func changeuid_t (
   fun () ->
 let g = open_guestfs ~identifier:"rhev_disk_create" () in
-(* For qcow2, override v2v-supplied compat option, because RHEL 6
- * nodes cannot handle qcow2 v3 (RHBZ#1145582).
- *)
-let compat = if format <> "qcow2" then compat else Some "0.10" in
 g#disk_create ?backingfile ?backingformat ?preallocation ?compat
   ?clustersize path format size;
 (* Make it sufficiently writable so that possibly root, or
diff --git a/v2v/output_vdsm.ml b/v2v/output_vdsm.ml
index 7cd94c0..6771daf 100644
--- a/v2v/output_vdsm.ml
+++ b/v2v/output_vdsm.ml
@@ -30,6 +30,7 @@ type vdsm_params = {
   vol_uuids : string list;
   vm_uuid : string;
   ovf_output : string;
+  compat : string;
 }
 
 class output_vdsm os vdsm_params output_alloc =
@@ -37,13 +38,16 @@ object
   inherit output
 
   method as_options =
-sprintf "-o vdsm -os %s%s%s --vdsm-vm-uuid %s --vdsm-ovf-output %s" os
+sprintf "-o vdsm -os %s%s%s --vdsm-vm-uuid %s --vdsm-ovf-output %s%s" os
   (String.concat ""
  (List.map (sprintf " --vdsm-image-uuid %s") vdsm_params.image_uuids))
   (String.concat ""
  (List.map (sprintf " --vdsm-vol-uuid %s") vdsm_params.vol_uuids))
   vdsm_params.vm_uuid
   vdsm_params.ovf_output
+  (match vdsm_params.compat

[Libguestfs] [PATCH v3] v2v: -o vdsm: Add --vdsm-compat flag.

2016-12-07 Thread Richard W.M. Jones 
v3:

Change the flag from --vdsm-compat-11 to --vdsm-compat=1.1

Also the --machine-readable output has changed.

I have also added a test.

Rich.

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

Re: [Libguestfs] [PATCH 0/5] Improve inspection of /usr filesystems

2016-12-07 Thread Richard W.M. Jones 
On Wed, Dec 07, 2016 at 10:49:44AM +0100, Pino Toscano wrote:
> On Wednesday, 7 December 2016 08:29:54 CET Richard W.M. Jones wrote:
> > On Tue, Dec 06, 2016 at 04:29:17PM +0100, Pino Toscano wrote:
> > > Hi,
> > > 
> > > this patch series improves the way /usr filesystems are handled: tag
> > > them appropriately, so later on we can find them and merge results they
> > > contain directly back for the root filesystem.
> > > 
> > > The series includes also a new private debug API, and its usage to fix
> > > the resolution of /dev/mapper/.. devices found in fstab; without it,
> > > LVM /usr filesystems are not recognized as belonging to their roots.
> > > Maybe a better API for this could be added, but since it's something
> > > only related to the appliance then can stay internal for now. (Better
> > > suggestions always welcome, of course.)
> > 
> > Firstly, I still think we need:
> > 
> >   [PATCH 1/2] inspect: fstab: Canonicalize paths appearing in fstab.
> > 
> > although it's not related to the current patch series.
> 
> This was not an issue in the data of RHBZ#140147, do you have an example
> of guest with such paths in fstab? So far I never seen one myself.

I don't, but it could happen and I see no problem with canonicalizing
these paths.  Having these "weird" paths in fstab causes the same
paths to be returned by guestfs_inspect_get_filesystems and might
cause problems for other API users.

> > >   daemon: debug: new "exists" subcommand
> > >   inspect: fix existance check of /dev/mapper devices
> > 
> > This is very ugly.  We really shouldn't be calling a debug API from
> > inspection code.
> 
> As mentioned above, any better suggestion is welcome. I didn't want to
> add a new public API for inspecting files in the appliance itself --
> what about something like internal_exists?

I'll follow up on the commit message.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

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

[Libguestfs] [PATCH 1/3] Document modules.

2016-12-07 Thread Richard W.M. Jones 
Add documentation for the modules making up supermin.

This is just documentation, there is no code change.
---
 src/Makefile.am |  9 +
 src/build.mli   | 23 +++
 src/chroot.mli  | 25 +
 src/dpkg.mli| 22 ++
 src/ext2.mli| 29 +
 src/ext2_initrd.mli | 31 +++
 src/kernel.mli  | 35 +++
 src/pacman.mli  | 22 ++
 src/prepare.mli | 23 +++
 src/rpm.mli | 22 ++
 10 files changed, 241 insertions(+)
 create mode 100644 src/build.mli
 create mode 100644 src/chroot.mli
 create mode 100644 src/dpkg.mli
 create mode 100644 src/ext2.mli
 create mode 100644 src/ext2_initrd.mli
 create mode 100644 src/kernel.mli
 create mode 100644 src/pacman.mli
 create mode 100644 src/prepare.mli
 create mode 100644 src/rpm.mli

diff --git a/src/Makefile.am b/src/Makefile.am
index 767117f..c0e9656 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -52,14 +52,23 @@ SOURCES = \
package_handler.ml \
package_handler.mli \
rpm.ml \
+   rpm.mli \
dpkg.ml \
+   dpkg.mli \
pacman.ml \
+   pacman.mli \
prepare.ml \
+   prepare.mli \
chroot.ml \
+   chroot.mli \
kernel.ml \
+   kernel.mli \
ext2_initrd.ml \
+   ext2_initrd.mli \
ext2.ml \
+   ext2.mli \
build.ml \
+   build.mli \
supermin.ml
 
 # Can't use filter for this because of automake brokenness.
diff --git a/src/build.mli b/src/build.mli
new file mode 100644
index 000..6feb4f8
--- /dev/null
+++ b/src/build.mli
@@ -0,0 +1,23 @@
+(* supermin 5
+ * Copyright (C) 2009-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
+ *)
+
+(** Implements the [--build] subcommand. *)
+
+val build : int -> (bool * string option * Types.format * string * string 
option * string * bool * int64 option * bool) -> string list -> string -> unit
+(** [build debug (args...) inputs outputdir] performs the
+[supermin --build] subcommand. *)
diff --git a/src/chroot.mli b/src/chroot.mli
new file mode 100644
index 000..5bd81c7
--- /dev/null
+++ b/src/chroot.mli
@@ -0,0 +1,25 @@
+(* supermin 5
+ * Copyright (C) 2009-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
+ *)
+
+(** Implements [--build -f chroot]. *)
+
+val build_chroot : int -> Package_handler.file list -> string -> string option 
-> unit
+(** [build_chroot debug files outputdir packagelist_file] copies the
+list of [files] into the chroot at [outputdir].  The optional
+[packagelist] controls creation of [/packagelist] within the
+chroot. *)
diff --git a/src/dpkg.mli b/src/dpkg.mli
new file mode 100644
index 000..c21327c
--- /dev/null
+++ b/src/dpkg.mli
@@ -0,0 +1,22 @@
+(* supermin 5
+ * Copyright (C) 2009-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

[Libguestfs] [PATCH 0/3] Miscellaneous improvements to supermin.

2016-12-07 Thread Richard W.M. Jones 
Document what each module does, using *.mli files.

Remove the --dtb option, it's obsolete.

Rename modules according to their purpose.

Rich.

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

Re: [Libguestfs] [PATCH] v2v: Rename RHEV to RHV throughout.

2016-12-07 Thread Richard W.M. Jones 
On Wed, Dec 07, 2016 at 11:19:47AM +0100, Pino Toscano wrote:
> On Wednesday, 7 December 2016 10:10:36 CET Richard W.M. Jones wrote:
> > On Wed, Dec 07, 2016 at 10:56:05AM +0100, Pino Toscano wrote:
> > > On Wednesday, 7 December 2016 08:34:35 CET Richard W.M. Jones wrote:
> > > > On Tue, Dec 06, 2016 at 04:41:52PM +0100, Pino Toscano wrote:
> > > > > On Thursday, 1 December 2016 14:35:07 CET Richard W.M. Jones wrote:
> > > > > > You can now use -o rhv (-o rhev is supported for compatibility).
> > > > > 
> > > > > This LGTM -- the only concern is that "output:rhev" will disappear 
> > > > > from
> > > > > the machine-readable output (and thus potentially breaking users).
> > > > > I have PoC for handling better aliases for input & output modules,
> > > > > I will polish and submit it.
> > > > 
> > > > Hopefully they are parsing the --machine-readable output and so this
> > > > won't be a problem :-)
> > > 
> > > Well exactly: newer virt-v2v with "older" ovirt/rhv will not see
> > > output:rhev anymore and thus not enable the VM import from VMware/etc.
> > 
> > I think you mean newer virt-p2v and older virt-v2v?
> 
> No, I mean other users of v2v which looks at the machine-readable
> output. Not sure whether ovirt/rhv is one of them, but others might.

RHV is just using RPM version dependencies.  It doesn't look
at --machine-readable at all.  I don't know about other consumers.

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/2] libvirt: read secrets of disks (RHBZ#1392798)

2016-12-07 Thread Richard W.M. Jones 
On Wed, Nov 16, 2016 at 12:59:39PM +0100, Pino Toscano wrote:
> Read also the secrets associated to disks ( tag within ),
> so qemu can properly open them later on.

Looks good, ACK.

Rich.


>  src/libvirt-domain.c | 130 
> +++
>  1 file changed, 122 insertions(+), 8 deletions(-)
> 
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index baab307..696a264 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -33,6 +33,8 @@
>  #include 
>  #include 
>  
> +#include "base64.h"
> +
>  #include "guestfs.h"
>  #include "guestfs-internal.h"
>  #include "guestfs-internal-actions.h"
> @@ -40,7 +42,7 @@
>  #if defined(HAVE_LIBVIRT)
>  
>  static xmlDocPtr get_domain_xml (guestfs_h *g, virDomainPtr dom);
> -static ssize_t for_each_disk (guestfs_h *g, virConnectPtr conn, xmlDocPtr 
> doc, int (*f) (guestfs_h *g, const char *filename, const char *format, int 
> readonly, const char *protocol, char *const *server, const char *username, 
> void *data), void *data);
> +static ssize_t for_each_disk (guestfs_h *g, virConnectPtr conn, xmlDocPtr 
> doc, int (*f) (guestfs_h *g, const char *filename, const char *format, int 
> readonly, const char *protocol, char *const *server, const char *username, 
> const char *secret, void *data), void *data);
>  static int libvirt_selinux_label (guestfs_h *g, xmlDocPtr doc, char 
> **label_rtn, char **imagelabel_rtn);
>  static char *filename_from_pool (guestfs_h *g, virConnectPtr conn, const 
> char *pool_nane, const char *volume_name);
>  static bool xPathObjectIsEmpty (xmlXPathObjectPtr obj);
> @@ -95,8 +97,12 @@ guestfs_impl_add_domain (guestfs_h *g, const char 
> *domain_name,
>  return -1;
>}
>  
> -  /* Connect to libvirt, find the domain. */
> -  conn = guestfs_int_open_libvirt_connection (g, libvirturi, VIR_CONNECT_RO);
> +  /* Connect to libvirt, find the domain.  We cannot open the connection
> +   * in read-only mode (VIR_CONNECT_RO), as that kind of connection
> +   * is considered untrusted, and thus libvirt will prevent to read
> +   * the values of secrets.
> +   */
> +  conn = guestfs_int_open_libvirt_connection (g, libvirturi, 0);
>if (!conn) {
>  err = virGetLastError ();
>  error (g, _("could not connect to libvirt (code %d, domain %d): %s"),
> @@ -163,7 +169,7 @@ guestfs_impl_add_domain (guestfs_h *g, const char 
> *domain_name,
>return r;
>  }
>  
> -static int add_disk (guestfs_h *g, const char *filename, const char *format, 
> int readonly, const char *protocol, char *const *server, const char 
> *username, void *data);
> +static int add_disk (guestfs_h *g, const char *filename, const char *format, 
> int readonly, const char *protocol, char *const *server, const char 
> *username, const char *secret, void *data);
>  static int connect_live (guestfs_h *g, virDomainPtr dom);
>  
>  enum readonlydisk {
> @@ -325,7 +331,7 @@ static int
>  add_disk (guestfs_h *g,
>const char *filename, const char *format, int readonly_in_xml,
>const char *protocol, char *const *server, const char *username,
> -  void *datavp)
> +  const char *secret, void *datavp)
>  {
>struct add_disk_data *data = datavp;
>/* Copy whole struct so we can make local changes: */
> @@ -382,6 +388,10 @@ add_disk (guestfs_h *g,
>  optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_USERNAME_BITMASK;
>  optargs.username = username;
>}
> +  if (secret) {
> +optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_SECRET_BITMASK;
> +optargs.secret = secret;
> +  }
>  
>return guestfs_add_drive_opts_argv (g, filename, );
>  }
> @@ -475,7 +485,7 @@ for_each_disk (guestfs_h *g,
>   const char *filename, const char *format,
>   int readonly,
>   const char *protocol, char *const *server,
> - const char *username,
> + const char *username, const char *secret,
>   void *data),
> void *data)
>  {
> @@ -504,7 +514,7 @@ for_each_disk (guestfs_h *g,
>if (nodes != NULL) {
>  nr_nodes = nodes->nodeNr;
>  for (i = 0; i < nr_nodes; ++i) {
> -  CLEANUP_FREE char *type = NULL, *filename = NULL, *format = NULL, 
> *protocol = NULL, *username = NULL;
> +  CLEANUP_FREE char *type = NULL, *filename = NULL, *format = NULL, 
> *protocol = NULL, *username = NULL, *secret = NULL;
>CLEANUP_FREE_STRING_LIST char **server = NULL;
>CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xptype = NULL;
>CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpformat = NULL;
> @@ -517,6 +527,7 @@ for_each_disk (guestfs_h *g,
>CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpvolume = NULL;
>int readonly;
>int t;
> +  virErrorPtr err;
>  
>/* Change the context to the current  node.
> * DV advises to reset this before each search since older versions of
> @@ -569,8 +580,111

Re: [Libguestfs] [PATCH 1/2] libvirt: un-duplicate XPath code

2016-12-07 Thread Richard W.M. Jones 
On Wed, Nov 16, 2016 at 12:59:38PM +0100, Pino Toscano wrote:
> Move the checks for empty xmlXPathObjectPtr, and for extracting the
> result string out of it, to a new helper functions.
> 
> This is just code motion, there should be no behaviour changes.
> ---
>  src/libvirt-domain.c | 122 
> +--
>  1 file changed, 50 insertions(+), 72 deletions(-)
> 
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 4d4142d..baab307 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -43,6 +43,8 @@ static xmlDocPtr get_domain_xml (guestfs_h *g, virDomainPtr 
> dom);
>  static ssize_t for_each_disk (guestfs_h *g, virConnectPtr conn, xmlDocPtr 
> doc, int (*f) (guestfs_h *g, const char *filename, const char *format, int 
> readonly, const char *protocol, char *const *server, const char *username, 
> void *data), void *data);
>  static int libvirt_selinux_label (guestfs_h *g, xmlDocPtr doc, char 
> **label_rtn, char **imagelabel_rtn);
>  static char *filename_from_pool (guestfs_h *g, virConnectPtr conn, const 
> char *pool_nane, const char *volume_name);
> +static bool xPathObjectIsEmpty (xmlXPathObjectPtr obj);
> +static char *xPathObjectGetString (xmlDocPtr doc, xmlXPathObjectPtr obj);

To be consistent with the rest of the code, xpath_object_is_empty etc.

ACK with that changed.

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/2] daemon: allow to change the labels of swap partitions

2016-12-07 Thread Richard W.M. Jones 
On Fri, Nov 25, 2016 at 11:22:26AM +0100, Pino Toscano wrote:
> ---
>  daemon/daemon.h  |  1 +
>  daemon/labels.c  |  3 +++
>  daemon/swap.c| 21 +
>  generator/actions.ml |  4 
>  4 files changed, 29 insertions(+)
> 
> diff --git a/daemon/daemon.h b/daemon/daemon.h
> index 79a5288..2379e31 100644
> --- a/daemon/daemon.h
> +++ b/daemon/daemon.h
> @@ -254,6 +254,7 @@ extern int64_t ntfs_minimum_size (const char *device);
>  
>  /*-- in swap.c --*/
>  extern int swap_set_uuid (const char *device, const char *uuid);
> +extern int swap_set_label (const char *device, const char *label);
>  
>  /* ordinary daemon functions use these to indicate errors
>   * NB: you don't need to prefix the string with the current command,
> diff --git a/daemon/labels.c b/daemon/labels.c
> index 20f27cb..aaa3eaf 100644
> --- a/daemon/labels.c
> +++ b/daemon/labels.c
> @@ -85,6 +85,9 @@ do_set_label (const mountable_t *mountable, const char 
> *label)
>else if (STREQ (vfs_type, "xfs"))
>  r = xfslabel (mountable->device, label);
>  
> +  else if (STREQ (vfs_type, "swap"))
> +r = swap_set_label (mountable->device, label);
> +
>else
>  NOT_SUPPORTED (-1, "don't know how to set the label for '%s' 
> filesystems",
> vfs_type);
> diff --git a/daemon/swap.c b/daemon/swap.c
> index 9d7839e..028bc1e 100644
> --- a/daemon/swap.c
> +++ b/daemon/swap.c
> @@ -239,3 +239,24 @@ swap_set_uuid (const char *device, const char *uuid)
>  
>return 0;
>  }
> +
> +int
> +swap_set_label (const char *device, const char *label)
> +{
> +  int r;
> +  CLEANUP_FREE char *err = NULL;
> +
> +  if (strlen (label) > SWAP_LABEL_MAX) {
> +reply_with_error ("%s: Linux swap labels are limited to %d bytes",
> +  label, SWAP_LABEL_MAX);
> +return -1;
> +  }
> +
> +  r = command (NULL, , str_swaplabel, "-L", label, device, NULL);
> +  if (r == -1) {
> +reply_with_error ("%s", err);
> +return -1;
> +  }
> +
> +  return 0;
> +}
> diff --git a/generator/actions.ml b/generator/actions.ml
> index 43de38b..5e0356f 100644
> --- a/generator/actions.ml
> +++ b/generator/actions.ml
> @@ -10304,6 +10304,10 @@ when trying to set the label.
>  
>  The label is limited to 11 bytes.
>  
> +=item swap
> +
> +The label is limited to 16 bytes.
> +
>  =back
>  
>  If there is no support for changing the label
> -- 
> 2.7.4
> 
> ___
> Libguestfs mailing list
> Libguestfs@redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs

Obvious improvement, ACK.

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

[Libguestfs] [PATCH v2 1/2] inspect: fix existance check of /dev/mapper devices

2016-12-07 Thread Pino Toscano 
When checking for the existance of /dev/mapper devices found in the
fstab of a filesystem, using guestfs_exists means they are checked as
files in the guest, while they really appear as devices on the
appliance. Instead, try the lvm name resolution anyway, and ignore them
when they are reported as missing.

Thanks to: Richard W.M. Jones.

Fixes commit 96b6504b09461aeb6850bb2e7b870a0a4c2f5edf.
---
 src/inspect-fs-unix.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/inspect-fs-unix.c b/src/inspect-fs-unix.c
index c833304..1e1cf5d 100644
--- a/src/inspect-fs-unix.c
+++ b/src/inspect-fs-unix.c
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #ifdef HAVE_ENDIAN_H
 #include 
@@ -1820,7 +1821,7 @@ resolve_fstab_device (guestfs_h *g, const char *spec, 
Hash_table *md_map,
   char *type, *slice, *disk, *part;
   int r;
 
-  if (STRPREFIX (spec, "/dev/mapper/") && guestfs_exists (g, spec) > 0) {
+  if (STRPREFIX (spec, "/dev/mapper/")) {
 /* LVM2 does some strange munging on /dev/mapper paths for VGs and
  * LVs which contain '-' character:
  *
@@ -1831,7 +1832,17 @@ resolve_fstab_device (guestfs_h *g, const char *spec, 
Hash_table *md_map,
  * This makes it impossible to reverse those paths directly, so
  * we have implemented lvm_canonical_lv_name in the daemon.
  */
+guestfs_push_error_handler (g, NULL, NULL);
 device = guestfs_lvm_canonical_lv_name (g, spec);
+guestfs_pop_error_handler (g);
+if (device == NULL) {
+  if (guestfs_last_errno (g) == ENOENT) {
+/* Ignore devices that don't exist. (RHBZ#811872) */
+  } else {
+guestfs_int_error_errno (g, guestfs_last_errno (g), "%s", 
guestfs_last_error (g));
+return NULL;
+  }
+}
   }
   else if (match3 (g, spec, re_xdev, , , )) {
 r = resolve_fstab_device_xdev (g, type, disk, part, );
-- 
2.7.4

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

[Libguestfs] [PATCH v2 2/2] inspect: gather info from /usr filesystems as well (RHBZ#1401474)

2016-12-07 Thread Pino Toscano 
Flag the filesystems for Linux /usr properly as USR role, and detect
some data out of it, like the distro information from an os-release
(if present), and the architecture (since the binaries used for our
architecture check will be available there only).

Later on, collect the results in a way similar to what is done for
CoreOS: for each non-CoreOS root, try to find its /usr filesystem, and
if found then merge what is missing from root; in the last case, also
override the distro inspection data (version, product name) if available
in /usr.
---
 src/guestfs-internal.h |  1 +
 src/inspect-fs-unix.c  | 26 ++
 src/inspect-fs.c   |  6 ++--
 src/inspect.c  | 74 ++
 4 files changed, 105 insertions(+), 2 deletions(-)

diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
index d10191d..fbbfb90 100644
--- a/src/guestfs-internal.h
+++ b/src/guestfs-internal.h
@@ -864,6 +864,7 @@ extern void guestfs_int_merge_fs_inspections (guestfs_h *g, 
struct inspect_fs *d
 
 /* inspect-fs-unix.c */
 extern int guestfs_int_check_linux_root (guestfs_h *g, struct inspect_fs *fs);
+extern int guestfs_int_check_linux_usr (guestfs_h *g, struct inspect_fs *fs);
 extern int guestfs_int_check_freebsd_root (guestfs_h *g, struct inspect_fs 
*fs);
 extern int guestfs_int_check_netbsd_root (guestfs_h *g, struct inspect_fs *fs);
 extern int guestfs_int_check_openbsd_root (guestfs_h *g, struct inspect_fs 
*fs);
diff --git a/src/inspect-fs-unix.c b/src/inspect-fs-unix.c
index 1e1cf5d..fcc39e5 100644
--- a/src/inspect-fs-unix.c
+++ b/src/inspect-fs-unix.c
@@ -787,6 +787,32 @@ guestfs_int_check_linux_root (guestfs_h *g, struct 
inspect_fs *fs)
   return 0;
 }
 
+/* The currently mounted device looks like a Linux /usr. */
+int
+guestfs_int_check_linux_usr (guestfs_h *g, struct inspect_fs *fs)
+{
+  int r;
+
+  fs->type = OS_TYPE_LINUX;
+  fs->role = OS_ROLE_USR;
+
+  if (guestfs_is_file_opts (g, "/lib/os-release",
+GUESTFS_IS_FILE_OPTS_FOLLOWSYMLINKS, 1, -1) > 0) {
+r = parse_os_release (g, fs, "/lib/os-release");
+if (r == -1)/* error */
+  return -1;
+if (r == 1) /* ok - detected the release from this file */
+  goto skip_release_checks;
+  }
+
+ skip_release_checks:;
+
+  /* Determine the architecture. */
+  check_architecture (g, fs);
+
+  return 0;
+}
+
 /* The currently mounted device is known to be a FreeBSD root. */
 int
 guestfs_int_check_freebsd_root (guestfs_h *g, struct inspect_fs *fs)
diff --git a/src/inspect-fs.c b/src/inspect-fs.c
index 1951678..9f7630b 100644
--- a/src/inspect-fs.c
+++ b/src/inspect-fs.c
@@ -245,8 +245,10 @@ check_filesystem (guestfs_h *g, const char *mountable,
is_dir_bin &&
is_dir_share &&
guestfs_is_dir (g, "/local") > 0 &&
-   guestfs_is_file (g, "/etc/fstab") == 0)
-;
+   guestfs_is_file (g, "/etc/fstab") == 0) {
+if (guestfs_int_check_linux_usr (g, fs) == -1)
+  return -1;
+  }
   /* CoreOS /usr? */
   else if (is_dir_bin &&
is_dir_share &&
diff --git a/src/inspect.c b/src/inspect.c
index 9055226..f6d0840 100644
--- a/src/inspect.c
+++ b/src/inspect.c
@@ -31,6 +31,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #ifdef HAVE_ENDIAN_H
 #include 
@@ -46,6 +47,8 @@ COMPILE_REGEXP (re_primary_partition, 
"^/dev/(?:h|s|v)d.[1234]$", 0)
 
 static void check_for_duplicated_bsd_root (guestfs_h *g);
 static void collect_coreos_inspection_info (guestfs_h *g);
+static void collect_linux_inspection_info (guestfs_h *g);
+static void collect_linux_inspection_info_for (guestfs_h *g, struct inspect_fs 
*root);
 
 /**
  * The main inspection API.
@@ -88,6 +91,12 @@ guestfs_impl_inspect_os (guestfs_h *g)
*/
   check_for_duplicated_bsd_root (g);
 
+  /* For Linux guests with a separate /usr filesyste, merge some of the
+   * inspected information in that partition to the inspect_fs struct
+   * of the root filesystem.
+   */
+  collect_linux_inspection_info (g);
+
   /* At this point we have, in the handle, a list of all filesystems
* found and data about each one.  Now we assemble the list of
* filesystems which are root devices and return that to the user.
@@ -149,6 +158,71 @@ collect_coreos_inspection_info (guestfs_h *g)
 }
 
 /**
+ * Traverse through the filesystems and find the /usr filesystem for
+ * the specified C: if found, merge its basic inspection details
+ * to the root when they were set (i.e. because the /usr had os-release
+ * or other ways to identify the OS).
+ */
+static void
+collect_linux_inspection_info_for (guestfs_h *g, struct inspect_fs *root)
+{
+  size_t i;
+  struct inspect_fs *usr = NULL;
+
+  for (i = 0; i < g->nr_fses; ++i) {
+struct inspect_fs *fs = >fses[i];
+size_t j;
+
+if (!(fs->distro == root->distro || fs->distro == OS_DISTRO_UNKNOWN) ||
+fs->role != OS_ROLE_USR)
+  continue;
+
+for (j = 0; j < root->nr_fstab; ++j) {
+  if

Re: [Libguestfs] [PATCH 2/3] Split internal stuff out of guestfs.h

2016-12-07 Thread Richard W.M. Jones 

Oh and just make "guestfs-internal.h" include those other two
files, and drop explicit includes of guestfs-internal-actions.h
everywhere.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

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

Re: [Libguestfs] [PATCH 2/3] Split internal stuff out of guestfs.h

2016-12-07 Thread Richard W.M. Jones 
On Tue, Nov 08, 2016 at 02:30:19PM +0100, Pino Toscano wrote:
> Create a new guestfs-private.h header, and move there the definitions of
> all the actions with visibility VInternal, and all the private structs:
> they are not meant to be used, not even seen, outside of the library.
> 
> Include the new header where needed, which means also a couple of places
> outside the library (but they are tests, so it is acceptable for now).
> 
> The result of this is mostly motion of function and structs
> declarations.

OK I understand why this change is made, but the naming of the new
header is super-confusing.  We already have guestfs-internal.h and
guestfs-internal-actions.h (containing guestfs_impl_*), and now we're
adding guestfs-private.h containing guestfs_internal_* functions.

Hmm.

So the idea is fine, but renaming things to make them more sensible.

How about guestfs-internal-impl.h for guestfs_impl_*
and guestfs-internal-apis.h for guestfs_internal_* ?

Rich.

>  .gitignore  |  1 +
>  generator/c.ml  | 52 
> +
>  generator/c.mli |  1 +
>  generator/main.ml   |  1 +
>  generator/tests_c_api.ml|  1 +
>  src/available.c |  1 +
>  src/drives.c|  1 +
>  src/file.c  |  1 +
>  src/handle.c|  1 +
>  src/inspect-fs.c|  1 +
>  src/journal.c   |  1 +
>  src/mountable.c |  1 +
>  src/tsk.c   |  1 +
>  tests/mountable/test-internal-parse-mountable.c |  1 +
>  tests/regressions/rhbz914931.c  |  1 +
>  15 files changed, 58 insertions(+), 8 deletions(-)
> 
> diff --git a/.gitignore b/.gitignore
> index 633b39d..8235f77 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -505,6 +505,7 @@ Makefile.in
>  /src/guestfs_protocol.c
>  /src/guestfs_protocol.h
>  /src/guestfs_protocol.x
> +/src/guestfs-private.h
>  /src/guestfs-structs.pod
>  /src/libguestfs.3
>  /src/libguestfs.pc
> diff --git a/generator/c.ml b/generator/c.ml
> index f0df5ea..4497c48 100644
> --- a/generator/c.ml
> +++ b/generator/c.ml
> @@ -39,7 +39,13 @@ let is_public { visibility = v } = match v with
>| VPublic | VPublicNoFish | VStateTest | VDebug -> true
>| VBindTest | VInternal -> false
>  
> -let is_private f = not (is_public f)
> +let is_private { visibility = v } = match v with
> +  | VBindTest -> true
> +  | VPublic | VPublicNoFish | VStateTest | VDebug | VInternal -> false
> +
> +let is_internal { visibility = v } = match v with
> +  | VInternal -> true
> +  | VPublic | VPublicNoFish | VStateTest | VDebug | VBindTest -> false
>  
>  let public_functions_sorted =
>List.filter is_public (actions |> sort)
> @@ -47,6 +53,9 @@ let public_functions_sorted =
>  let private_functions_sorted =
>List.filter is_private (actions |> sort)
>  
> +let internal_functions_sorted =
> +  List.filter is_internal (actions |> sort)
> +
>  (* Generate a C function prototype. *)
>  let rec generate_prototype ?(extern = true) ?(static = false)
>  ?(semicolon = true)
> @@ -607,13 +616,6 @@ extern GUESTFS_DLL_PUBLIC void *guestfs_next_private 
> (guestfs_h *g, const char *
>  
>generate_all_headers private_functions_sorted;
>  
> -  pr "\
> -/* Private structures. */
> -
> -";
> -
> -  generate_all_structs internal_structs;
> -
>  pr "\
>  
>  #endif /* End of GUESTFS_PRIVATE. */
> @@ -650,6 +652,34 @@ pr "\
>  #endif /* GUESTFS_H_ */
>  "
>  
> +(* Generate the guestfs-private.h file. *)
> +and generate_guestfs_private_h () =
> +  generate_header CStyle LGPLv2plus;
> +
> +  pr "\
> +#ifndef GUESTFS_PRIVATE_H_
> +#define GUESTFS_PRIVATE_H_
> +
> +#include \"guestfs.h\"
> +
> +/* Private functions. */
> +
> +";
> +
> +  generate_all_headers internal_functions_sorted;
> +
> +  pr "\
> +/* Private structures. */
> +
> +";
> +
> +  generate_all_structs internal_structs;
> +
> +  pr "\
> +
> +#endif /* GUESTFS_PRIVATE_H_ */
> +"
> +
>  (* The structures are carefully written to have exactly the same
>   * in-memory format as the XDR structures that we use on the wire to
>   * the daemon.  The reason for creating copies of these structures
> @@ -856,6 +886,7 @@ and generate_client_structs_free () =
>  #include 
>  
>  #include \"guestfs.h\"
> +#include \"guestfs-private.h\"
>  #include \"guestfs-internal.h\"
>  #include \"guestfs_protocol.h\"
>  
> @@ -905,6 +936,7 @@ and generate_client_structs_compare () =
>  #include 
>  
>  #include \"guestfs.h\"
> +#include \"guestfs-private.h\"
>  #include \"guestfs-internal.h\"
>  
>  ";
> @@ -990,6 +1022,7 @@ and generate_client_structs_copy () =
>  #include 
>  
>  #include \"guestfs.h\"
> +#include \"guestfs-private.h\"
>  #include \"guestfs-internal.h\"
>  
>

Re: [Libguestfs] [PATCH 2/2] resize: shrink/expand swap partitions

2016-12-07 Thread Richard W.M. Jones 
On Fri, Nov 25, 2016 at 11:22:27AM +0100, Pino Toscano wrote:
> Handle the swap partition on their own, rebuilding them using the
> existing UUID and label.
>
>  resize/resize.ml   | 35 ---
>  resize/virt-resize.pod |  8 
>  2 files changed, 36 insertions(+), 7 deletions(-)
> 
> diff --git a/resize/resize.ml b/resize/resize.ml
> index 7d06f18..59ee5bf 100644
> --- a/resize/resize.ml
> +++ b/resize/resize.ml
> @@ -60,6 +60,7 @@ and partition_content =
>| ContentPV of int64   (* physical volume (size of PV) *)
>| ContentFS of string * int64  (* mountable filesystem (FS type, FS size) 
> *)
>| ContentExtendedPartition (* MBR extended partition *)
> +  | ContentSwap  (* Swap partition *)
>  and partition_operation =
>| OpCopy   (* copy it as-is, no resizing *)
>| OpIgnore (* ignore it (create on target, but don't
> @@ -104,11 +105,13 @@ and string_of_partition_content = function
>| ContentPV sz -> sprintf "LVM PV (%Ld bytes)" sz
>| ContentFS (fs, sz) -> sprintf "filesystem %s (%Ld bytes)" fs sz
>| ContentExtendedPartition -> "extended partition"
> +  | ContentSwap -> "swap"
>  and string_of_partition_content_no_size = function
>| ContentUnknown -> "unknown data"
>| ContentPV _ -> "LVM PV"
>| ContentFS (fs, _) -> sprintf "filesystem %s" fs
>| ContentExtendedPartition -> "extended partition"
> +  | ContentSwap -> "swap"
>  
>  (* Data structure describing LVs on the source disk.  This is only
>   * used if the user gave the --lv-expand option.
> @@ -130,6 +133,7 @@ let debug_logvol lv =
>  
>  type expand_content_method =
>| PVResize | Resize2fs | NTFSResize | BtrfsFilesystemResize | XFSGrowFS
> +  | Mkswap
>  
>  let string_of_expand_content_method = function
>| PVResize -> s_"pvresize"
> @@ -137,6 +141,7 @@ let string_of_expand_content_method = function
>| NTFSResize -> s_"ntfsresize"
>| BtrfsFilesystemResize -> s_"btrfs-filesystem-resize"
>| XFSGrowFS -> s_"xfs_growfs"
> +  | Mkswap -> s_"mkswap"
>  
>  type unknown_filesystems_mode =
>| UnknownFsIgnore
> @@ -414,6 +419,8 @@ read the man page virt-resize(1).
>  let fs = g#vfs_type dev in
>  if fs = "unknown" then
>ContentUnknown
> +else if fs = "swap" then
> +  ContentSwap
>  else if fs = "LVM2_member" then (
>let rec loop = function
>  | [] ->
> @@ -531,7 +538,7 @@ read the man page virt-resize(1).
>  assert (
>match typ with
>| ContentPV _ | ContentExtendedPartition -> false
> -  | ContentUnknown | ContentFS _ -> true
> +  | ContentUnknown | ContentFS _ | ContentSwap -> true
>  );
>  
>  { lv_name = name; lv_type = typ; lv_operation = LVOpNone }
> @@ -558,6 +565,7 @@ read the man page virt-resize(1).
>| ContentFS (("xfs"), _) when !xfs_available -> true
>| ContentFS _ -> false
>| ContentExtendedPartition -> false
> +  | ContentSwap -> true
>  else
>fun _ -> false
>  
> @@ -572,6 +580,7 @@ read the man page virt-resize(1).
>| ContentFS (("xfs"), _) when !xfs_available -> XFSGrowFS
>| ContentFS _ -> assert false
>| ContentExtendedPartition -> assert false
> +  | ContentSwap -> Mkswap
>  else
>fun _ -> assert false
>in
> @@ -665,6 +674,7 @@ read the man page virt-resize(1).
>  | ContentExtendedPartition ->
>error (f_"%s: This extended partition contains logical partitions 
> which might be damaged by shrinking it.  If you want to shrink this 
> partition, you need to use the '--resize-force' option, but that could 
> destroy logical partitions within this partition.  (This error came from '%s' 
> option on the command line.)")
>  name option
> +| ContentSwap -> ()
>);
>  
>p.p_operation <- OpResize newsize
> @@ -831,7 +841,8 @@ read the man page virt-resize(1).
>  (match p.p_type with
>  | ContentUnknown
>  | ContentPV _
> -| ContentExtendedPartition -> ()
> +| ContentExtendedPartition
> +| ContentSwap -> ()
>  | ContentFS (fs, _) ->
>error (f_"unknown/unavailable method for expanding the %s 
> filesystem on %s")
>  fs p.p_name
> @@ -848,7 +859,8 @@ read the man page virt-resize(1).
>  (match lv.lv_type with
>  | ContentUnknown
>  | ContentPV _
> -| ContentExtendedPartition -> ()
> +| ContentExtendedPartition
> +| ContentSwap -> ()
>  | ContentFS (fs, _) ->
>error (f_"unknown/unavailable method for expanding the %s 
> filesystem on %s")
>  fs lv.lv_name;
> @@ -886,7 +898,8 @@ read the man page virt-resize(1).
>(match p.p_type with
>|

[Libguestfs] [PATCH v2 0/2] Improve inspection of /usr filesystems

2016-12-07 Thread Pino Toscano 
Hi,

this patch series improves the way /usr filesystems are handled: tag
them appropriately, so later on we can find them and merge results they
contain directly back for the root filesystem.

Changes in v2:
- removed patches #1 and #2, already pushed
- drop patch #3, no more needed
- replace patch #4 with a better suggestion from Rich
- change if into assert in patch #5

Thanks,

Pino Toscano (2):
  inspect: fix existance check of /dev/mapper devices
  inspect: gather info from /usr filesystems as well (RHBZ#1401474)

 src/guestfs-internal.h |  1 +
 src/inspect-fs-unix.c  | 39 +-
 src/inspect-fs.c   |  6 ++--
 src/inspect.c  | 74 ++
 4 files changed, 117 insertions(+), 3 deletions(-)

-- 
2.7.4

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

Re: [Libguestfs] [PATCH v2 0/2] Improve inspection of /usr filesystems

2016-12-07 Thread Richard W.M. Jones 
On Wed, Dec 07, 2016 at 01:27:54PM +0100, Pino Toscano wrote:
> Hi,
> 
> this patch series improves the way /usr filesystems are handled: tag
> them appropriately, so later on we can find them and merge results they
> contain directly back for the root filesystem.

ACK series.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

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

[Libguestfs] [PATCH v3 4/6] mllib: modify nsplit to take optional noempty and count arguments

2016-12-07 Thread Tomáš Golembiovský 
Added two new optional arguments to nsplit:

* noempty: if set to false empty elements are not stored in the returned
  list. The default is to keep the empty elements

* count: specifies how many splits to perform; negative count
  (the default) means do as many splits as possible

Signed-off-by: Tomáš Golembiovský 
---
 mllib/common_utils.ml  | 12 +---
 mllib/common_utils.mli | 12 ++--
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/mllib/common_utils.ml b/mllib/common_utils.ml
index 78618f5..2d373f9 100644
--- a/mllib/common_utils.ml
+++ b/mllib/common_utils.ml
@@ -92,15 +92,21 @@ module String = struct
 s' ^ s2 ^ replace s'' s1 s2
   )
 
-let rec nsplit sep str =
+let rec nsplit ?(noempty = true) ?(count = -1) sep str =
   let len = length str in
   let seplen = length sep in
   let i = find str sep in
-  if i = -1 then [str]
+  if i = -1 || count = 0 then [str]
   else (
 let s' = sub str 0 i in
 let s'' = sub str (i+seplen) (len-i-seplen) in
-s' :: nsplit sep s''
+let elem, count =
+  if s' = "" && noempty then
+[], count
+  else
+[s'], if count > 0 then count-1 else count
+in
+elem @ nsplit ~noempty:noempty ~count:count sep s''
   )
 
 let split sep str =
diff --git a/mllib/common_utils.mli b/mllib/common_utils.mli
index ad43345..bcd459d 100644
--- a/mllib/common_utils.mli
+++ b/mllib/common_utils.mli
@@ -64,9 +64,17 @@ module String : sig
 val replace : string -> string -> string -> string
 (** [replace str s1 s2] replaces all instances of [s1] appearing in
 [str] with [s2]. *)
-val nsplit : string -> string -> string list
+val nsplit : ?noempty:bool -> ?count:int -> string -> string -> string list
 (** [nsplit sep str] splits [str] into multiple strings at each
-separator [sep]. *)
+separator [sep].
+
+If [noempty] is set to true empty elements are not included in the
+list. By default empty elements are kept.
+
+If [count] is specified it says how many splits to perform. I.e. the
+returned array will have at most [count]+1 elements. Negative [count]
+(the default) means do as many splits as possible.
+*)
 val split : string -> string -> string * string
 (** [split sep str] splits [str] at the first occurrence of the
 separator [sep], returning the part before and the part after.
-- 
2.10.2

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

[Libguestfs] [PATCH v3 6/6] v2v: ova: don't extract files from OVA if it's not needed

2016-12-07 Thread Tomáš Golembiovský 
We don't have to always extract all files from the OVA archive. The OVA,
as defined in the standard, is plain tar. We can work directly over the
tar archive if we use correct 'offset' and 'size' options when defining
the backing file for QEMU. This puts much lower requirement on available
disk space.

Since the virt-v2v behaviour for OVA input now depends on QEMU version
available this affects some of the tests. The affected tests will have
two *.expect files and the expected result also has to depend on the
QEMU used.

Signed-off-by: Tomáš Golembiovský 
---
 test-data/test-utils.sh |  20 
 v2v/Makefile.am |   1 +
 v2v/input_ova.ml| 180 +---
 v2v/test-v2v-i-ova-formats.sh   |   5 +-
 v2v/test-v2v-i-ova-subfolders.expected2 |  18 
 v2v/test-v2v-i-ova-subfolders.sh|  13 ++-
 v2v/test-v2v-i-ova-tar.expected |  18 
 v2v/test-v2v-i-ova-tar.expected2|  18 
 v2v/test-v2v-i-ova-tar.ovf  | 138 
 v2v/test-v2v-i-ova-tar.sh   |  71 +
 v2v/test-v2v-i-ova-two-disks.expected2  |  19 
 v2v/test-v2v-i-ova-two-disks.sh |  13 ++-
 12 files changed, 491 insertions(+), 23 deletions(-)
 create mode 100644 v2v/test-v2v-i-ova-subfolders.expected2
 create mode 100644 v2v/test-v2v-i-ova-tar.expected
 create mode 100644 v2v/test-v2v-i-ova-tar.expected2
 create mode 100644 v2v/test-v2v-i-ova-tar.ovf
 create mode 100755 v2v/test-v2v-i-ova-tar.sh
 create mode 100644 v2v/test-v2v-i-ova-two-disks.expected2

diff --git a/test-data/test-utils.sh b/test-data/test-utils.sh
index 86a5aaf..04e8333 100755
--- a/test-data/test-utils.sh
+++ b/test-data/test-utils.sh
@@ -54,3 +54,23 @@ do_sha256 ()
   ;;
   esac
 }
+
+# Returns 0 if QEMU version is greater or equal to the arguments
+qemu_is_version() {
+if [ $# -ne 2 ] ; then
+echo "Usage: $0  " >&2
+return 3
+fi
+
+QV=$(expr match "$(qemu-img --version)" 'qemu-img version 
\([0-9]\+\.[0-9]\+\)')
+[ -z "$QV" ] && return 2
+
+QMAJ=$(echo "$QV" | cut -d. -f1)
+QMIN=$(echo "$QV" | cut -d. -f2)
+
+if [ \( $QMAJ -gt $1 \) -o \( $QMAJ -eq $1 -a $QMIN -ge $2 \) ] ; then
+return 0
+fi
+
+return 1
+}
diff --git a/v2v/Makefile.am b/v2v/Makefile.am
index 5e3c3eb..621ba10 100644
--- a/v2v/Makefile.am
+++ b/v2v/Makefile.am
@@ -258,6 +258,7 @@ TESTS_ENVIRONMENT = $(top_builddir)/run --test
 
 TESTS = \
test-v2v-docs.sh \
+   test-v2v-i-ova-tar.sh \
test-v2v-i-ova-formats.sh \
test-v2v-i-ova-gz.sh \
test-v2v-i-ova-subfolders.sh \
diff --git a/v2v/input_ova.ml b/v2v/input_ova.ml
index 85954a3..b7828b4 100644
--- a/v2v/input_ova.ml
+++ b/v2v/input_ova.ml
@@ -39,17 +39,23 @@ object
 
   method source () =
 
-let untar ?(format = "") file outdir =
-  let cmd = [ "tar"; sprintf "-x%sf" format; file; "-C"; outdir ] in
+(* Untar part or all files from tar archive. If [paths] is specified it is
+ * a list of paths in the tar archive.
+ *)
+let untar ?(format = "") ?paths file outdir =
+  let cmd =
+[ "tar"; sprintf "-x%sf" format; file; "-C"; outdir ]
+@ (match paths with None -> [] | Some p -> p)
+  in
   if run_command cmd <> 0 then
 error (f_"error unpacking %s, see earlier error messages") ova in
 
 (* Extract ova file. *)
-let exploded =
+let exploded, partial =
   (* The spec allows a directory to be specified as an ova.  This
* is also pretty convenient.
*)
-  if is_directory ova then ova
+  if is_directory ova then ova, false
   else (
 let uncompress_head zcat file =
   let cmd = sprintf "%s %s" zcat (quote file) in
@@ -67,11 +73,56 @@ object
 
   tmpfile in
 
+(* Untar only ovf and manifest from the archive *)
+let untar_metadata ova outdir =
+  let files =
+external_command (sprintf "tar -tf %s" (Filename.quote ova)) in
+  let files =
+filter_map (fun f ->
+  if Filename.check_suffix f ".ovf" ||
+  Filename.check_suffix f ".mf" then
+Some f
+  else None
+) files in
+  untar ~paths:files ova outdir in
+
+let qemu_img_version () =
+  let cmd = "qemu-img --version" in
+  let lines = external_command cmd in
+  match lines with
+  | [] -> error ("'qemu-img --version' returned no output")
+  | line :: _ ->
+  let rex = Str.regexp "qemu-img version 
\\([0-9]+\\)\\.\\([0-9]+\\)" in
+  if Str.string_match rex line 0 then (
+try
+  int_of_string (Str.matched_group 1 line),
+  int_of_string (Str.matched_group 2 line)
+with Failure _ ->
+  warning (f_"failed to parse qemu-img version(%S),

[Libguestfs] [PATCH v3 3/6] v2v: ova: move the untar function

2016-12-07 Thread Tomáš Golembiovský 
Move the untar function so it can be used later in the code.

Signed-off-by: Tomáš Golembiovský 
---
 v2v/input_ova.ml | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/v2v/input_ova.ml b/v2v/input_ova.ml
index 61930f0..85954a3 100644
--- a/v2v/input_ova.ml
+++ b/v2v/input_ova.ml
@@ -38,6 +38,12 @@ object
   method as_options = "-i ova " ^ ova
 
   method source () =
+
+let untar ?(format = "") file outdir =
+  let cmd = [ "tar"; sprintf "-x%sf" format; file; "-C"; outdir ] in
+  if run_command cmd <> 0 then
+error (f_"error unpacking %s, see earlier error messages") ova in
+
 (* Extract ova file. *)
 let exploded =
   (* The spec allows a directory to be specified as an ova.  This
@@ -61,11 +67,6 @@ object
 
   tmpfile in
 
-let untar ?(format = "") file outdir =
-  let cmd = [ "tar"; sprintf "-x%sf" format; file; "-C"; outdir ] in
-  if run_command cmd <> 0 then
-error (f_"error unpacking %s, see earlier error messages") ova in
-
 match detect_file_type ova with
 | `Tar ->
   (* Normal ovas are tar file (not compressed). *)
-- 
2.10.2

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

[Libguestfs] [PATCH v3 0/6] Import directly from OVA tar archive if possible

2016-12-07 Thread Tomáš Golembiovský 
v3: Addressed Pino's comments, namely:
- input_ova.ml
  - untar takes list of paths
  - renamed untar_partial to untar_metadata
  - replaced uggly regex with nsplit
- tests
  - test changes are part of the main commit
  - renamed test-data/guestfs-hashsums.sh to test-data/test-utils.sh
  - renamed qemu_version to qemu_is_version and moved it to
test-data/test-utils.sh
  - normalize paths in expect files


v2:
- rewritten the tar invocations, the output processing is now done in
  OcaML rather than with a shell code; it turned out to be easier and
  more readable than I have feared.
- added QEMU version check
- addressed Pino's comments
- changed tests; the expected result is now based on the QEMU used
  during testing

This series is related to the problem of inefficient import of OVA
files. The needed enhancements of QEMU were merged into the codebase and
should be available in QEMU 2.8. From there we can use 'size' and
'offset' options in raw driver to tell QEMU to use only subset of a file
as an image.

The patch set is more or less complete. The only outstanding issue is
the missing detection of sparse files inside tar. But this can be done
in a separate patch. As pointed out before I didn't find a way how to
detect that by using the tar tool only and would probably require use of
some external library.

The first three patches are just preparation. The main work is in patch
four. Last patch fixes the tests.


Tomáš Golembiovský (6):
  mllib: compute checksum of file inside tar
  v2v: ova: don't detect compressed disks, read the OVF instead
  v2v: ova: move the untar function
  mllib: modify nsplit to take optional noempty and count arguments
  tests: rename guestfs-hashsums.sh to test-utils.sh
  v2v: ova: don't extract files from OVA if it's not needed

 mllib/checksums.ml   |  11 +-
 mllib/checksums.mli  |   7 +-
 mllib/common_utils.ml|  12 +-
 mllib/common_utils.mli   |  12 +-
 test-data/Makefile.am|   2 +-
 test-data/{guestfs-hashsums.sh => test-utils.sh} |  20 +++
 tests/qemu/qemu-liveness.sh  |   2 +-
 tests/qemu/qemu-snapshot-isolation.sh|   2 +-
 v2v/Makefile.am  |   1 +
 v2v/input_ova.ml | 211 ---
 v2v/test-v2v-i-ova-formats.sh|   7 +-
 v2v/test-v2v-i-ova-gz.ovf|   2 +-
 v2v/test-v2v-i-ova-gz.sh |   2 +-
 v2v/test-v2v-i-ova-subfolders.expected2  |  18 ++
 v2v/test-v2v-i-ova-subfolders.sh |  15 +-
 v2v/test-v2v-i-ova-tar.expected  |  18 ++
 v2v/test-v2v-i-ova-tar.expected2 |  18 ++
 v2v/test-v2v-i-ova-tar.ovf   | 138 +++
 v2v/test-v2v-i-ova-tar.sh|  71 
 v2v/test-v2v-i-ova-two-disks.expected2   |  19 ++
 v2v/test-v2v-i-ova-two-disks.sh  |  15 +-
 v2v/test-v2v-i-ova.sh|   2 +-
 v2v/test-v2v-in-place.sh |   2 +-
 23 files changed, 553 insertions(+), 54 deletions(-)
 rename test-data/{guestfs-hashsums.sh => test-utils.sh} (73%)
 create mode 100644 v2v/test-v2v-i-ova-subfolders.expected2
 create mode 100644 v2v/test-v2v-i-ova-tar.expected
 create mode 100644 v2v/test-v2v-i-ova-tar.expected2
 create mode 100644 v2v/test-v2v-i-ova-tar.ovf
 create mode 100755 v2v/test-v2v-i-ova-tar.sh
 create mode 100644 v2v/test-v2v-i-ova-two-disks.expected2

-- 
2.10.2

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