Libguestfs - May 2022 - [PATCH v2v 2/2] convert: Remove /dev/mapper/osprober-* devices left around by grub2

On 05/17/22 12:59, Richard W.M. Jones wrote:
> These devices can be left around by grub2 when it runs the osprober
> tool after we run ?/sbin/grub2-mkconfig -o /boot/grub2/grub.cfg?.
> They are read-only mirrors of existing filesystems.  These confuse
> later steps in conversion, specifically fstrim.
> 
> Reported-by: Ming Xie
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2003503
> ---
>  convert/linux_bootloaders.ml | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/convert/linux_bootloaders.ml b/convert/linux_bootloaders.ml
> index a70b65a41c..7c5fb0be3f 100644
> --- a/convert/linux_bootloaders.ml
> +++ b/convert/linux_bootloaders.ml
> @@ -345,7 +345,14 @@ object (self)
>    method remove_console = self#grub2_update_console ~remove:true
>  
>    method update () > -    ignore (g#command [| grub2_mkconfig_cmd;
"-o"; grub_config |])
> +    ignore (g#command [| grub2_mkconfig_cmd; "-o"; grub_config
|]);
> +
> +    (* Grub2 runs osprober which sometimes leaves around read-only
> +     * device-mapper maps covering existing filesystems.  These
> +     * confuse later steps (especially fstrim).  So just delete
> +     * any if found. (RHBZ#2003503).
> +     *)
> +    ignore (g#command [| "bash"; "-c"; "rm -f
/dev/mapper/osprober-*" |])
>  
>    method get_config_file () >      grub_config
> 
Hmm... I'm not sure why this is better than "dmsetup remove", but
it
does not really matter. We need these nodes to disappear only for the
remainder of the conversion -- so command that effects that suffices!

Reviewed-by: Laszlo Ersek <lersek at redhat.com>

Thanks!
Laszlo

On Tue, May 17, 2022 at 01:10:32PM +0200, Laszlo Ersek
wrote:
> On 05/17/22 12:59, Richard W.M. Jones wrote:
> > These devices can be left around by grub2 when it runs the osprober
> > tool after we run ?/sbin/grub2-mkconfig -o /boot/grub2/grub.cfg?.
> > They are read-only mirrors of existing filesystems.  These confuse
> > later steps in conversion, specifically fstrim.
> > 
> > Reported-by: Ming Xie
> > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2003503
> > ---
> >  convert/linux_bootloaders.ml | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/convert/linux_bootloaders.ml
b/convert/linux_bootloaders.ml
> > index a70b65a41c..7c5fb0be3f 100644
> > --- a/convert/linux_bootloaders.ml
> > +++ b/convert/linux_bootloaders.ml
> > @@ -345,7 +345,14 @@ object (self)
> >    method remove_console = self#grub2_update_console ~remove:true
> >  
> >    method update () > > -    ignore (g#command [|
grub2_mkconfig_cmd; "-o"; grub_config |])
> > +    ignore (g#command [| grub2_mkconfig_cmd; "-o";
grub_config |]);
> > +
> > +    (* Grub2 runs osprober which sometimes leaves around read-only
> > +     * device-mapper maps covering existing filesystems.  These
> > +     * confuse later steps (especially fstrim).  So just delete
> > +     * any if found. (RHBZ#2003503).
> > +     *)
> > +    ignore (g#command [| "bash"; "-c"; "rm
-f /dev/mapper/osprober-*" |])
> >  
> >    method get_config_file () > >      grub_config
> > 
> 
> Hmm... I'm not sure why this is better than "dmsetup remove",
but it
> does not really matter. We need these nodes to disappear only for the
> remainder of the conversion -- so command that effects that suffices!
> 
> Reviewed-by: Laszlo Ersek <lersek at redhat.com>
Yeah, I'm not really sure what I was thinking there.  I certainly
*meant* to use dmsetup remove, but maybe ... it's accidentally better?

The problem with running dmsetup is that we have to run the guest
command, which might not exist.

Fun fact I found when investigating this.  Upstream os-prober dropped
the whole device mapper / dmsetup / osprober-linux-* stuff back in
2017.  The current code doesn't mention osprober-linux-* at all.
So I guess the version in this RHEL 8 guest is quite old.

Since I'm still testing the (above) version of this patch, let's see
if "rm" actually works before doing any further work on this.

Rich.

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

On Tue, May 17, 2022 at 01:10:32PM +0200, Laszlo Ersek
wrote:
> On 05/17/22 12:59, Richard W.M. Jones wrote:
> > These devices can be left around by grub2 when it runs the osprober
> > tool after we run ?/sbin/grub2-mkconfig -o /boot/grub2/grub.cfg?.
> > They are read-only mirrors of existing filesystems.  These confuse
> > later steps in conversion, specifically fstrim.
> > 
> > Reported-by: Ming Xie
> > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2003503
> > ---
> >  convert/linux_bootloaders.ml | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/convert/linux_bootloaders.ml
b/convert/linux_bootloaders.ml
> > index a70b65a41c..7c5fb0be3f 100644
> > --- a/convert/linux_bootloaders.ml
> > +++ b/convert/linux_bootloaders.ml
> > @@ -345,7 +345,14 @@ object (self)
> >    method remove_console = self#grub2_update_console ~remove:true
> >  
> >    method update () > > -    ignore (g#command [|
grub2_mkconfig_cmd; "-o"; grub_config |])
> > +    ignore (g#command [| grub2_mkconfig_cmd; "-o";
grub_config |]);
> > +
> > +    (* Grub2 runs osprober which sometimes leaves around read-only
> > +     * device-mapper maps covering existing filesystems.  These
> > +     * confuse later steps (especially fstrim).  So just delete
> > +     * any if found. (RHBZ#2003503).
> > +     *)
> > +    ignore (g#command [| "bash"; "-c"; "rm
-f /dev/mapper/osprober-*" |])
> >  
> >    method get_config_file () > >      grub_config
> > 
I'd like to make this change (on top):

-    ignore (g#command [| "bash"; "-c"; "rm -f
/dev/mapper/osprober-*" |])
+    ignore (g#command [| "sh"; "-c"; "rm -f
/dev/mapper/osprober-linux-*" |])

Bash might in theory not exist. I guess it's unlikely, but POSIX
defines 'sh' so we know that must exist.  It's not as if we're
using
any bash features here.

The other part of the change is that the device nodes created by the
old & buggy osprober were always called "osprober-linux-*", so we
may
as well be more specific about that.

I still cannot reproduce this bug locally.  I can force grub2-mkconfig
to run -- creating a guest like this is sufficient:

$ virt-builder rhel-8.4 --edit '/etc/sysconfig/grub:
s|^GRUB_CMDLINE_LINUX="|GRUB_CMDLINE_LINUX="resume=/dev/sda1 |'

However it doesn't reproduce the bug observed.

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