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

[Richard W.M. Jones]

On Fri, Oct 06, 2017 at 04:08:14PM +0200, Pino Toscano wrote:
> On Friday, 6 October 2017 12:11:08 CEST Richard W.M. Jones wrote:
> > 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.
> 
> As I said in my first reply to this email, this is the actual issue.
> 
> > > 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:
> 
> As I said already, using --no-application & --no-icon gives already the
> same; also, banning *BSD systems entirely from detailed inspections
> means that:
> a) whenever UFS is fixed, we will probably forget to remove this code
> b) if UFS is not the filesystem used, then this issue does not happen
>    at all, and just no extra information are returned (but this is a
>    different issue)
>
> Also, the problem is not specific to virt-inspector: any tool that
> relies on inspection, and then mounts the partitions according to that,
> will face the same issue -- e.g. virt-ls, virt-cat, virt-customize, etc.
> 
> That's why I do not think adding a workaround only in virt-inspector
> makes much sense per-se, since it will not deal with general issue wrt
> UFS partitions.

OK these are fair points.

I'll withdraw this patch.  I still don't really know how to fix
this properly but I'll think about it some more.

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