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

---
 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.
    */
-  if (inspect_apps || inspect_icon) {
+  if ((inspect_apps || inspect_icon) && !is_bsd) {
     inspect_mount_root (g, root);
 
     if (inspect_apps)
-- 
2.13.2

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.

The trick here is what check_for_filesystem_on in daemon/inspect_fs.ml
does, by trying few mount options for ufs filesystems: usually one of
those attempts succeeds, so we extract the information from that
partition.  Later on in virt-inspector, though, these extra mount
options are not retained, so it fails.

The two possible options I had in mind were:
a) duplicate the same mount logic also in virt-inspector, which is ugly,
duplicates code, etc
b) save these extra mount options, exposing them via a new inspect_*
   API: IMHO it is ugly as well, since it would "make official" the
   extra mount attempts hack tried in inspect_fs.ml
but both are ugly, in one way or another, so I did not implement either
of them.

Anyway, IMHO this patch is not the correct approach.

-- 
Pino Toscano

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