[PATCH v2v 2/2] convert: Remove /dev/mapper/osprober-* devices left around by grub2
We were invoking grub2-mkconfig in the same way in two different places. This just refactors one of the calls so it uses the #update method. It should make no difference. --- convert/linux_bootloaders.ml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/convert/linux_bootloaders.ml b/convert/linux_bootloaders.ml index 0bde2c0d73..a70b65a41c 100644 --- a/convert/linux_bootloaders.ml +++ b/convert/linux_bootloaders.ml @@ -333,7 +333,7 @@ object (self) g#aug_save (); try - ignore (g#command [| grub2_mkconfig_cmd; "-o"; grub_config |]) + self#update () with G.Error msg -> warning (f_"could not rebuild grub2 configuration file (%s). This may mean that grub output will not be sent to the serial port, but otherwise should be harmless. Original error message: %s") -- 2.35.1
On 05/17/22 12:59, Richard W.M. Jones wrote:
We were invoking grub2-mkconfig in the same way in two different places. This just refactors one of the calls so it uses the #update method. It should make no difference. --- convert/linux_bootloaders.ml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/convert/linux_bootloaders.ml b/convert/linux_bootloaders.ml index 0bde2c0d73..a70b65a41c 100644 --- a/convert/linux_bootloaders.ml +++ b/convert/linux_bootloaders.ml @@ -333,7 +333,7 @@ object (self) g#aug_save ();
try - ignore (g#command [| grub2_mkconfig_cmd; "-o"; grub_config |]) + self#update () with G.Error msg -> warning (f_"could not rebuild grub2 configuration file (%s). This may mean that grub output will not be sent to the serial port, but otherwise should be harmless. Original error message: %s")
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
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 -- 2.35.1
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@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@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 05/17/22 13:18, Richard W.M. Jones wrote:
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@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.
Wow, indeed! ... Earlier I had trouble finding the upstream repository for os-prober, but now I've found it: https://salsa.debian.org/installer-team/os-prober.git and the commit you mean is most likely b760c18ffec7 ("Remove code using device mapper.", 2017-04-30) It names three debian bug reports, and sure enough, the last one of those is what we're hitting: "Leaves stale device-mapper entry" https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=853163 (The bug report also refers to "dmsetup remove".) Similar Fedora bug reports ("CLOSED EOL"): os-prober gives device-mapper errors https://bugzilla.redhat.com/show_bug.cgi?id=1600778 grub2-mkconfig does not work correctly with os-prober https://bugzilla.redhat.com/show_bug.cgi?id=1744693 Then upstream fixed the bug by ripping out the device mapper code. :/ As replacement, the upstream commit recommends "grub-mount" -- which I guess is not available in RHEL8? That's right, in dist-git, on the rhel-8.7.0 branch at least, the "grub.macros" file passes "--disable-grub-mount" to the BIOS and EFI builds both. :/ This option seems to go back to historical dist-git commit 31004e676e8c ("Rebase to upstream snapshot.", 2013-06-07), which was even part of rhel-7.0! In other words, the dmsetup leak was never per se fixed (despite the many reports), and upstream's recommended alternative has never been enabled in RHEL. Stellar!
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.
OK! 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
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
On 05/17/22 15:09, Richard W.M. Jones wrote:
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.
Sure; both are good ideas.
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.
I think it's OK; you did test the fix, just on an image from someone else. And we have confirmation from multiple independent tickets that this symptom exists. Thanks! Laszlo
Thanks - upstream in: https://github.com/libguestfs/virt-v2v/commit/4ad661b545f476f8dfa00f3cdc28be... https://github.com/libguestfs/virt-v2v/commit/e06cf5b5dcbfd3f0a798eaf75c8778... 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 Tue, May 17, 2022 at 11:59:26AM +0100, Richard W.M. Jones wrote:
Still testing this one as well ...
The version (with rm) fixes the bug for me. 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
participants (2)
-
Laszlo Ersek -
Richard W.M. Jones