Libguestfs - May 2022 - [PATCH v2v] convert: Ignore /dev/mapper/osprober-* devices when trimming

On 05/17/22 09:53, Richard W.M. Jones wrote:
> On Tue, May 17, 2022 at 07:54:01AM +0200, Laszlo Ersek wrote:
>> On 05/16/22 17:46, Richard W.M. Jones wrote:
>>> These devices can be left around by either grub2 or the osprober
tool.
>>> They are read-only mirrors of existing filesystems and it appears
we
>>> can safely ignore them.
>>>
>>> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2003503
>>> ---
>>>  convert/convert.ml | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/convert/convert.ml b/convert/convert.ml
>>> index 87fca7252b..997f6b08bd 100644
>>> --- a/convert/convert.ml
>>> +++ b/convert/convert.ml
>>> @@ -194,10 +194,16 @@ and do_fstrim g inspect >>>    (*
Get all filesystems. *)
>>>    let fses = g#list_filesystems () in
>>>  
>>> +  (* Ignore unknown/swap devices. *)
>>>    let fses = List.filter_map (
>>>      function (_, ("unknown"|"swap")) ->
None | (dev, _) -> Some dev
>>>    ) fses in
>>>  
>>> +  (* Ignore filesystems left around by osprober (RHBZ#2003503). *)
>>> +  let fses >>> +    List.filter (fun dev -> not
(String.is_prefix dev "/dev/mapper/osprober-"))
>>> +                fses in
>>> +
>>>    (* Trim the filesystems. *)
>>>    List.iter (
>>>      fun dev ->
>>>
>>
>> I've read up on os-prober:
>>
>> https://www.thegeekdiary.com/how-to-disable-os-prober-in-centos-rhel-7/
>> https://github.com/campadrenalin/os-prober
>> http://joeyh.name/code/os-prober/
>>
>> and it's *absolutely infuriating* that os-prober itself, or
whatever
>> invokes os-prober, litters the system with block device nodes. In fact,
>> I don't even know how it is *possible* for a device node to be left
>> around under /dev/mapper -- I thought /dev/mapper/ would be
re-populated
>> *from zero* every time the system boots? Do we have something in the
>> appliance perhaps that *creates* these files?
> 
> As part of conversion we run ?/sbin/grub2-mkconfig -o /boot/grub2/grub.cfg?
> which runs osprober and that leaves the block devices around.  Later
> in conversion we list out all filesystems (ie. g#list_filesystems) and
> try to trim these, which is what was giving the warnings.
> 
> This is only a workaround for the original bug which seems to be in
> grub/osprober.
> 
>> I'm quite unhappy about ignoring these filesystems in
"do_fstrim"
>> *only*. If guestfs_list_filesystems() returns them in the first place,
>> won't that cause confusion for other libguestfs applications or
scripts?
> 
> Quite possibly, but I was fixing the smallest possible thing to make
> the bug go away.
> 
>> Do we expect the same filesystem to appear multiply in the
>> list-filesystems output?
> 
> During conversion, list_filesystems output is [tidied up]:
> 
> list_filesystems = [
>   "/dev/mapper/osprober-linux-rhel_bootp--73--199--22-root",
"xfs";
>   "/dev/mapper/osprober-linux-sda1", "xfs";
>   "/dev/sda1", "xfs";
>   "/dev/sda3", "xfs";
>   "/dev/rhel_bootp-73-199-22/root", "xfs";
>   "/dev/rhel_bootp-73-199-22/swap", "swap";
>   "/dev/rhel_bootp-73-199-6/root", "xfs";
>   "/dev/rhel_bootp-73-199-6/swap", "swap"
> ]
> 
> (/dev/mapper/osprober-linux-sda1 & /dev/sda1), and
> (/dev/mapper/osprober-linux-rhel_bootp--73--199--22-root &
/dev/rhel_bootp-73-199-22/root)
> are both duplicates.
> 
> It would be quite hard to deduplicate these however (unless you know
> some trick), since the /dev/mapper/osprober-* devices are just
> read-only device-mapper linear entries.  There's nothing apart from
> the name that easily connects them to the partitions and LVs that they
> cover (particularly the LV).
I agree that we should not add complex logic here.

Do you think it's feasible to *move* the name-based filtering from
virt-v2v's do_fstrim to the libguestfs daemon's list-filesystems API
implementation? I can't see a reason why the list-filesystems API should
*ever* return these /dev/mapper/osprober-* nodes.

IOW all I'm proposing is that we move the same workaround deeper in the
stack.

Thanks!
Laszlo
> 
>> I think this is a serious bug in some component different from
>> libguestfs / virt-v2v.
> 
> Without a doubt, it's a bug in grub2/osprober.
> 
>> Hmmm... After reading your comment
>> <https://bugzilla.redhat.com/show_bug.cgi?id=2003503#c12>, I
think we
>> absolutely need to report a bug for grub2. I'll contact the rhboot
team.
>>
>> I don't disagree that we have to work it around, but until we
understand
>> the problem better, I don't feel safe about filtering these nodes
out
>> only in virt-v2v's trimming function.
> 
> Sure, I'll leave this one for now.
> 
> Rich.
>

On Tue, May 17, 2022 at 11:48:34AM +0200, Laszlo Ersek
wrote:
> On 05/17/22 09:53, Richard W.M. Jones wrote:
> > On Tue, May 17, 2022 at 07:54:01AM +0200, Laszlo Ersek wrote:
> >> On 05/16/22 17:46, Richard W.M. Jones wrote:
> >>> These devices can be left around by either grub2 or the
osprober tool.
> >>> They are read-only mirrors of existing filesystems and it
appears we
> >>> can safely ignore them.
> >>>
> >>> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2003503
> >>> ---
> >>>  convert/convert.ml | 6 ++++++
> >>>  1 file changed, 6 insertions(+)
> >>>
> >>> diff --git a/convert/convert.ml b/convert/convert.ml
> >>> index 87fca7252b..997f6b08bd 100644
> >>> --- a/convert/convert.ml
> >>> +++ b/convert/convert.ml
> >>> @@ -194,10 +194,16 @@ and do_fstrim g inspect >
>>>    (* Get all filesystems. *)
> >>>    let fses = g#list_filesystems () in
> >>>  
> >>> +  (* Ignore unknown/swap devices. *)
> >>>    let fses = List.filter_map (
> >>>      function (_, ("unknown"|"swap"))
-> None | (dev, _) -> Some dev
> >>>    ) fses in
> >>>  
> >>> +  (* Ignore filesystems left around by osprober
(RHBZ#2003503). *)
> >>> +  let fses > >>> +    List.filter (fun dev ->
not (String.is_prefix dev "/dev/mapper/osprober-"))
> >>> +                fses in
> >>> +
> >>>    (* Trim the filesystems. *)
> >>>    List.iter (
> >>>      fun dev ->
> >>>
> >>
> >> I've read up on os-prober:
> >>
> >>
https://www.thegeekdiary.com/how-to-disable-os-prober-in-centos-rhel-7/
> >> https://github.com/campadrenalin/os-prober
> >> http://joeyh.name/code/os-prober/
> >>
> >> and it's *absolutely infuriating* that os-prober itself, or
whatever
> >> invokes os-prober, litters the system with block device nodes. In
fact,
> >> I don't even know how it is *possible* for a device node to be
left
> >> around under /dev/mapper -- I thought /dev/mapper/ would be
re-populated
> >> *from zero* every time the system boots? Do we have something in
the
> >> appliance perhaps that *creates* these files?
> > 
> > As part of conversion we run ?/sbin/grub2-mkconfig -o
/boot/grub2/grub.cfg?
> > which runs osprober and that leaves the block devices around.  Later
> > in conversion we list out all filesystems (ie. g#list_filesystems) and
> > try to trim these, which is what was giving the warnings.
> > 
> > This is only a workaround for the original bug which seems to be in
> > grub/osprober.
> > 
> >> I'm quite unhappy about ignoring these filesystems in
"do_fstrim"
> >> *only*. If guestfs_list_filesystems() returns them in the first
place,
> >> won't that cause confusion for other libguestfs applications
or scripts?
> > 
> > Quite possibly, but I was fixing the smallest possible thing to make
> > the bug go away.
> > 
> >> Do we expect the same filesystem to appear multiply in the
> >> list-filesystems output?
> > 
> > During conversion, list_filesystems output is [tidied up]:
> > 
> > list_filesystems = [
> >   "/dev/mapper/osprober-linux-rhel_bootp--73--199--22-root",
"xfs";
> >   "/dev/mapper/osprober-linux-sda1", "xfs";
> >   "/dev/sda1", "xfs";
> >   "/dev/sda3", "xfs";
> >   "/dev/rhel_bootp-73-199-22/root", "xfs";
> >   "/dev/rhel_bootp-73-199-22/swap", "swap";
> >   "/dev/rhel_bootp-73-199-6/root", "xfs";
> >   "/dev/rhel_bootp-73-199-6/swap", "swap"
> > ]
> > 
> > (/dev/mapper/osprober-linux-sda1 & /dev/sda1), and
> > (/dev/mapper/osprober-linux-rhel_bootp--73--199--22-root &
/dev/rhel_bootp-73-199-22/root)
> > are both duplicates.
> > 
> > It would be quite hard to deduplicate these however (unless you know
> > some trick), since the /dev/mapper/osprober-* devices are just
> > read-only device-mapper linear entries.  There's nothing apart
from
> > the name that easily connects them to the partitions and LVs that they
> > cover (particularly the LV).
> 
> I agree that we should not add complex logic here.
> 
> Do you think it's feasible to *move* the name-based filtering from
> virt-v2v's do_fstrim to the libguestfs daemon's list-filesystems
API
> implementation? I can't see a reason why the list-filesystems API
should
> *ever* return these /dev/mapper/osprober-* nodes.
> 
> IOW all I'm proposing is that we move the same workaround deeper in the
> stack.
Sure, that would work too, I'll come up with another patch soon.

Rich.
> Thanks!
> Laszlo
> 
> > 
> >> I think this is a serious bug in some component different from
> >> libguestfs / virt-v2v.
> > 
> > Without a doubt, it's a bug in grub2/osprober.
> > 
> >> Hmmm... After reading your comment
> >> <https://bugzilla.redhat.com/show_bug.cgi?id=2003503#c12>, I
think we
> >> absolutely need to report a bug for grub2. I'll contact the
rhboot team.
> >>
> >> I don't disagree that we have to work it around, but until we
understand
> >> the problem better, I don't feel safe about filtering these
nodes out
> >> only in virt-v2v's trimming function.
> > 
> > Sure, I'll leave this one for now.
> > 
> > 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

On 05/17/22 11:48, Laszlo Ersek wrote:
> Do you think it's feasible to *move* the name-based filtering from
> virt-v2v's do_fstrim to the libguestfs daemon's list-filesystems
API
> implementation? I can't see a reason why the list-filesystems API
should
> *ever* return these /dev/mapper/osprober-* nodes.
> 
> IOW all I'm proposing is that we move the same workaround deeper in the
> stack.
Scratch that -- that still allows the grub2/osprober bug to spread too far.

Once we get some info from the rhboot team on the nature of this
grub2/osprober bug, can we perhaps unmount and "dmsetup remove" these
/dev/mapper/osprober-* block devices, in v2v, right after we call
"grub2_mkconfig_cmd"? That would contain this issue tightly.

Thanks
Laszlo