Libguestfs - Oct 2017 - Re: [PATCH] inspector: Fix virt-inspector on *BSD guests (RHBZ#1144138).

On Thu, Oct 05, 2017 at 06:55:53PM +0200, Pino Toscano
wrote:
> On Thursday, 5 October 2017 17:36:09 CEST Richard W.M. Jones wrote:
> > ---
> >  inspector/inspector.c | 12 +++++++++++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> > 
> > diff --git a/inspector/inspector.c b/inspector/inspector.c
> > index 3583c61df..30d279987 100644
> > --- a/inspector/inspector.c
> > +++ b/inspector/inspector.c
> > @@ -347,6 +347,7 @@ output_root (xmlTextWriterPtr xo, char *root)
> >    char buf[32];
> >    char *canonical_root;
> >    size_t size;
> > +  int is_bsd;
> >  
> >    XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST
"operatingsystem"));
> >  
> > @@ -362,6 +363,10 @@ output_root (xmlTextWriterPtr xo, char *root)
> >    if (STRNEQ (str, "unknown"))
> >      XMLERROR (-1,
> >  	      xmlTextWriterWriteElement (xo, BAD_CAST "name",
BAD_CAST str));
> > +  is_bsd > > +    STREQ (str, "freebsd") ||
> > +    STREQ (str, "netbsd") ||
> > +    STREQ (str, "openbsd");
> >    free (str);
> >  
> >    str = guestfs_inspect_get_arch (g, root);
> > @@ -451,8 +456,13 @@ output_root (xmlTextWriterPtr xo, char *root)
> >  
> >    /* We need to mount everything up in order to read out the list of
> >     * applications and the icon, ie. everything below this point.
> > +   *
> > +   * XXX As a workaround for BSD guests, because the Linux kernel
> > +   * driver cannot just mount a UFS filesystem, we must disable this
> > +   * for all *BSD operating systems.  We cannot read the apps or icon
> > +   * from *BSD anyway.
> >     */
> 
> This is not true, libguestfs can actually read those properties.  The
> proof of that is running virt-inspector with --no-applications, and
> --no-icon shows the proper details (such as product name, mount points,
> host name, etc) of the guest.
I'm a bit confused by this comment - libguestfs inspection doesn't
know anything about apps and icons from *BSD as far as I can see.
This patch just (temporarily) makes it so that virt-inspection acts
like --no-applications --no-icon if it sees as *BSD guest, until we
think of a better way to do it.

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

On Thursday, 5 October 2017 23:49:26 CEST Richard W.M. Jones
wrote:
> On Thu, Oct 05, 2017 at 06:55:53PM +0200, Pino Toscano wrote:
> > On Thursday, 5 October 2017 17:36:09 CEST Richard W.M. Jones wrote:
> > > ---
> > >  inspector/inspector.c | 12 +++++++++++-
> > >  1 file changed, 11 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/inspector/inspector.c b/inspector/inspector.c
> > > index 3583c61df..30d279987 100644
> > > --- a/inspector/inspector.c
> > > +++ b/inspector/inspector.c
> > > @@ -347,6 +347,7 @@ output_root (xmlTextWriterPtr xo, char *root)
> > >    char buf[32];
> > >    char *canonical_root;
> > >    size_t size;
> > > +  int is_bsd;
> > >  
> > >    XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST
"operatingsystem"));
> > >  
> > > @@ -362,6 +363,10 @@ output_root (xmlTextWriterPtr xo, char
*root)
> > >    if (STRNEQ (str, "unknown"))
> > >      XMLERROR (-1,
> > >  	      xmlTextWriterWriteElement (xo, BAD_CAST "name",
BAD_CAST str));
> > > +  is_bsd > > > +    STREQ (str, "freebsd") ||
> > > +    STREQ (str, "netbsd") ||
> > > +    STREQ (str, "openbsd");
> > >    free (str);
> > >  
> > >    str = guestfs_inspect_get_arch (g, root);
> > > @@ -451,8 +456,13 @@ output_root (xmlTextWriterPtr xo, char
*root)
> > >  
> > >    /* We need to mount everything up in order to read out the
list of
> > >     * applications and the icon, ie. everything below this point.
> > > +   *
> > > +   * XXX As a workaround for BSD guests, because the Linux
kernel
> > > +   * driver cannot just mount a UFS filesystem, we must disable
this
> > > +   * for all *BSD operating systems.  We cannot read the apps or
icon
> > > +   * from *BSD anyway.
> > >     */
> > 
> > This is not true, libguestfs can actually read those properties.  The
> > proof of that is running virt-inspector with --no-applications, and
> > --no-icon shows the proper details (such as product name, mount
points,
> > host name, etc) of the guest.
> 
> I'm a bit confused by this comment
I thought this was a workaround to the fact that mounting UFS partitions
can fail, without particular mount options:

  $ ./run virt-inspector -a freebsd-10.qcow2
  libguestfs: error: mount_ro: mount exited with status 32: mount: /sysroot:
wrong fs type, bad option, bad superblock on /dev/sda2, missing codepage or
helper program, or other error.

(this happens in inspect_mount_root, when mounting it to get the icon
of the guest, and to list the applications)
> - libguestfs inspection doesn't
> know anything about apps and icons from *BSD as far as I can see.
> This patch just (temporarily) makes it so that virt-inspection acts
> like --no-applications --no-icon if it sees as *BSD guest, until we
> think of a better way to do it.
There are various guests in the same situation -- e.g. Gentoo, Pardus,
Slackware.  I do not see why we should special-case *BSD guests, since
at most (short of fixing the re-mount issue explained above) there will
be no information extracted.

-- 
Pino Toscano

On Fri, Oct 06, 2017 at 09:53:27AM +0200, Pino Toscano
wrote:
> On Thursday, 5 October 2017 23:49:26 CEST Richard W.M. Jones wrote:
> > On Thu, Oct 05, 2017 at 06:55:53PM +0200, Pino Toscano wrote:
> > > On Thursday, 5 October 2017 17:36:09 CEST Richard W.M. Jones
wrote:
> > > >    /* We need to mount everything up in order to read out
the list of
> > > >     * applications and the icon, ie. everything below this
point.
> > > > +   *
> > > > +   * XXX As a workaround for BSD guests, because the Linux
kernel
> > > > +   * driver cannot just mount a UFS filesystem, we must
disable this
> > > > +   * for all *BSD operating systems.  We cannot read the
apps or icon
> > > > +   * from *BSD anyway.
> > > >     */
> > > 
> > > This is not true, libguestfs can actually read those properties. 
The
> > > proof of that is running virt-inspector with --no-applications,
and
> > > --no-icon shows the proper details (such as product name, mount
points,
> > > host name, etc) of the guest.
> > 
> > I'm a bit confused by this comment
> 
> I thought this was a workaround to the fact that mounting UFS partitions
> can fail, without particular mount options:
> 
>   $ ./run virt-inspector -a freebsd-10.qcow2
>   libguestfs: error: mount_ro: mount exited with status 32: mount:
/sysroot: wrong fs type, bad option, bad superblock on /dev/sda2, missing
codepage or helper program, or other error.
Correct.
> (this happens in inspect_mount_root, when mounting it to get the icon
> of the guest, and to list the applications)
> 
> > - libguestfs inspection doesn't
> > know anything about apps and icons from *BSD as far as I can see.
> > This patch just (temporarily) makes it so that virt-inspection acts
> > like --no-applications --no-icon if it sees as *BSD guest, until we
> > think of a better way to do it.
> 
> There are various guests in the same situation -- e.g. Gentoo, Pardus,
> Slackware.
But Gentoo, Pardus & Slackware don't fail in the mount and so don't
need any workaround.
> I do not see why we should special-case *BSD guests, since
> at most (short of fixing the re-mount issue explained above) there will
> be no information extracted.
This isn't correct.  With this patch, instead of virt-inspector
printing nothing and failing with the mount error, you get the basic
information:

$ ./run virt-inspector /var/tmp/freebsd-11.1
<?xml version="1.0"?>
<operatingsystems>
  <operatingsystem>
    <root>/dev/sda5</root>
    <name>freebsd</name>
    <arch>x86_64</arch>
    <distro>freebsd</distro>
    <major_version>0</major_version>
    <minor_version>0</minor_version>
    <hostname></hostname>
    <mountpoints>
      <mountpoint dev="/dev/sda5">/</mountpoint>
    </mountpoints>
    <filesystems>
      <filesystem dev="/dev/sda5">
        <type>ufs</type>
        <uuid>59d658ab7d5c137f</uuid>
      </filesystem>
      <filesystem dev="/dev/sda6"/>
    </filesystems>
  </operatingsystem>
</operatingsystems>

(There is still a problem with major = minor = 0 but I'm going to try
to fix that separately).

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/