Laszlo Ersek
2021-Dec-15 15:31 UTC
[Libguestfs] [v2v PATCH v2] convert_linux: translate the first CD-ROM's references in boot conf files
If the only CD-ROM in "s_removables" is on an IDE controller, and the guest kernel represents it with a /dev/hdX device node, then convert references to this device node, in the boot config files, to /dev/cdrom. On the destination (after conversion), /dev/cdrom will point to whataver node we converted the CD-ROM to, masking a potential i440fx -> q35 (IDE -> SATA) board change. If the only CD-ROM is not on an IDE controller, or the guest is modern enough to represent the IDE CD-ROM as /dev/sr0, then perform no translation. Namely, /dev/sr0 survives a potential i440fx -> q35 (IDE -> SATA) board change intact. When multiple CD-ROMs exist, emit a warning, and attempt the conversion on the first CD-ROM, as a guess. This may be inexact, but we can't do better, because: - SATA, SCSI, and (on modern guests) IDE CD-ROMs are lumped together in the /dev/sr* namespace, on the source side, and "s_removable_slot" is useless for telling them apart, as we don't know the exact controller topology (and OS enumeration order); - after conversion: some OSes don't create /dev/cdrom* symlinks to all CD-ROMs, and even if multiple such symlinks are created, their order is basically random. Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1637857 Signed-off-by: Laszlo Ersek <lersek at redhat.com> --- Notes: v2: - Use List.length >= 2 rather than matching with three cons operations. [Rich] - At the same time, eliminate wrong precedence between "->" in a match, and ";". [Rich] - Match the first element of the cdrom list, and the contents of that element, in a single "match" expression. [Rich] - Not tested, due to the issue I described in <8b3ce08b-ea47-dc1c-f441-c8b91708bd6f at redhat.com> (cannot provide a mailing list URL because the archive seems to have stopped refreshing itself?) convert/convert_linux.ml | 19 +++++++++++++++++++ tests/test-v2v-cdrom.expected | 2 +- tests/test-v2v-cdrom.xml.in | 4 +++- tests/test-v2v-i-ova.xml | 2 +- 4 files changed, 24 insertions(+), 3 deletions(-) diff --git a/convert/convert_linux.ml b/convert/convert_linux.ml index 8dc648169dcb..d49ecec03aeb 100644 --- a/convert/convert_linux.ml +++ b/convert/convert_linux.ml @@ -1020,6 +1020,25 @@ let convert (g : G.guestfs) source inspect keep_serial_console _ "xvd" ^ drive_name i, block_prefix_after_conversion ^ drive_name i ) source.s_disks in + (* Check the first CD-ROM. If its controller is IDE, and the OS is RHEL<=5, + * then translate the CD-ROM from "/dev/hd[SLOT]" to "/dev/cdrom". See + * RHBZ#1637857 for details. + *) + let cdroms = List.filter + (fun removable -> removable.s_removable_type = CDROM) + source.s_removables in + if List.length cdroms >= 2 then + warning (f_"multiple CD-ROMs found; translation of CD-ROM references \ + may be inexact"); + let map = map @ + (match cdroms with + | { s_removable_controller = Some Source_IDE; + s_removable_slot = Some slot } :: _ + when family = `RHEL_family && inspect.i_major_version <= 5 -> + [("hd" ^ drive_name slot, "cdrom")] + | _ -> [] + ) in + if verbose () then ( eprintf "block device map:\n"; List.iter ( diff --git a/tests/test-v2v-cdrom.expected b/tests/test-v2v-cdrom.expected index 34d2bf5961b0..17bd152d8e64 100644 --- a/tests/test-v2v-cdrom.expected +++ b/tests/test-v2v-cdrom.expected @@ -4,5 +4,5 @@ </disk> <disk device='cdrom' type='file'> <driver name='qemu' type='raw'/> - <target dev='hdc' bus='ide'/> + <target dev='sdc' bus='sata'/> </disk> diff --git a/tests/test-v2v-cdrom.xml.in b/tests/test-v2v-cdrom.xml.in index 6bad5eab1cd4..a6e1e3f514d5 100644 --- a/tests/test-v2v-cdrom.xml.in +++ b/tests/test-v2v-cdrom.xml.in @@ -35,7 +35,9 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. <disk type='file' device='cdrom'> <driver name='qemu' type='raw'/> <source file='@abs_top_builddir@/test-data/phony-guests/blank-disk.img'/> - <!-- virt-v2v should preserve the device name and bus --> + <!-- virt-v2v should change the bus to sata, due to Windows 7 + triggering a machine type change from i440fx to q35. Beyond that, + virt-v2v should preserve the on-bus index. --> <target dev='hdc' bus='ide'/> </disk> </devices> diff --git a/tests/test-v2v-i-ova.xml b/tests/test-v2v-i-ova.xml index d7383905fdc0..9f3c1974243f 100644 --- a/tests/test-v2v-i-ova.xml +++ b/tests/test-v2v-i-ova.xml @@ -28,7 +28,7 @@ </disk> <disk device='cdrom' type='file'> <driver name='qemu' type='raw'/> - <target dev='hda' bus='ide'/> + <target dev='sda' bus='sata'/> </disk> <disk device='floppy' type='file'> <driver name='qemu' type='raw'/> -- 2.19.1.3.g30247aa5d201
Richard W.M. Jones
2021-Dec-16 09:45 UTC
[Libguestfs] [v2v PATCH v2] convert_linux: translate the first CD-ROM's references in boot conf files
On Wed, Dec 15, 2021 at 04:31:53PM +0100, Laszlo Ersek wrote:> If the only CD-ROM in "s_removables" is on an IDE controller, and the > guest kernel represents it with a /dev/hdX device node, then convert > references to this device node, in the boot config files, to /dev/cdrom. > On the destination (after conversion), /dev/cdrom will point to whataver > node we converted the CD-ROM to, masking a potential i440fx -> q35 (IDE -> > SATA) board change. > > If the only CD-ROM is not on an IDE controller, or the guest is modern > enough to represent the IDE CD-ROM as /dev/sr0, then perform no > translation. Namely, /dev/sr0 survives a potential i440fx -> q35 (IDE -> > SATA) board change intact. > > When multiple CD-ROMs exist, emit a warning, and attempt the conversion on > the first CD-ROM, as a guess. This may be inexact, but we can't do better, > because: > > - SATA, SCSI, and (on modern guests) IDE CD-ROMs are lumped together in > the /dev/sr* namespace, on the source side, and "s_removable_slot" is > useless for telling them apart, as we don't know the exact controller > topology (and OS enumeration order); > > - after conversion: some OSes don't create /dev/cdrom* symlinks to all > CD-ROMs, and even if multiple such symlinks are created, their order is > basically random. > > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1637857 > Signed-off-by: Laszlo Ersek <lersek at redhat.com> > --- > > Notes: > v2: > > - Use List.length >= 2 rather than matching with three cons operations. > [Rich] > > - At the same time, eliminate wrong precedence between "->" in a match, > and ";". [Rich] > > - Match the first element of the cdrom list, and the contents of that > element, in a single "match" expression. [Rich] > > - Not tested, due to the issue I described in > <8b3ce08b-ea47-dc1c-f441-c8b91708bd6f at redhat.com> (cannot provide a > mailing list URL because the archive seems to have stopped refreshing > itself?) > > convert/convert_linux.ml | 19 +++++++++++++++++++ > tests/test-v2v-cdrom.expected | 2 +- > tests/test-v2v-cdrom.xml.in | 4 +++- > tests/test-v2v-i-ova.xml | 2 +- > 4 files changed, 24 insertions(+), 3 deletions(-) > > diff --git a/convert/convert_linux.ml b/convert/convert_linux.ml > index 8dc648169dcb..d49ecec03aeb 100644 > --- a/convert/convert_linux.ml > +++ b/convert/convert_linux.ml > @@ -1020,6 +1020,25 @@ let convert (g : G.guestfs) source inspect keep_serial_console _ > "xvd" ^ drive_name i, block_prefix_after_conversion ^ drive_name i > ) source.s_disks in > > + (* Check the first CD-ROM. If its controller is IDE, and the OS is RHEL<=5, > + * then translate the CD-ROM from "/dev/hd[SLOT]" to "/dev/cdrom". See > + * RHBZ#1637857 for details. > + *) > + let cdroms = List.filter > + (fun removable -> removable.s_removable_type = CDROM) > + source.s_removables in > + if List.length cdroms >= 2 then > + warning (f_"multiple CD-ROMs found; translation of CD-ROM references \ > + may be inexact"); > + let map = map @ > + (match cdroms with > + | { s_removable_controller = Some Source_IDE; > + s_removable_slot = Some slot } :: _ > + when family = `RHEL_family && inspect.i_major_version <= 5 -> > + [("hd" ^ drive_name slot, "cdrom")] > + | _ -> [] > + ) in > + > if verbose () then ( > eprintf "block device map:\n"; > List.iter ( > diff --git a/tests/test-v2v-cdrom.expected b/tests/test-v2v-cdrom.expected > index 34d2bf5961b0..17bd152d8e64 100644 > --- a/tests/test-v2v-cdrom.expected > +++ b/tests/test-v2v-cdrom.expected > @@ -4,5 +4,5 @@ > </disk> > <disk device='cdrom' type='file'> > <driver name='qemu' type='raw'/> > - <target dev='hdc' bus='ide'/> > + <target dev='sdc' bus='sata'/> > </disk> > diff --git a/tests/test-v2v-cdrom.xml.in b/tests/test-v2v-cdrom.xml.in > index 6bad5eab1cd4..a6e1e3f514d5 100644 > --- a/tests/test-v2v-cdrom.xml.in > +++ b/tests/test-v2v-cdrom.xml.in > @@ -35,7 +35,9 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. > <disk type='file' device='cdrom'> > <driver name='qemu' type='raw'/> > <source file='@abs_top_builddir@/test-data/phony-guests/blank-disk.img'/> > - <!-- virt-v2v should preserve the device name and bus --> > + <!-- virt-v2v should change the bus to sata, due to Windows 7 > + triggering a machine type change from i440fx to q35. Beyond that, > + virt-v2v should preserve the on-bus index. --> > <target dev='hdc' bus='ide'/> > </disk> > </devices> > diff --git a/tests/test-v2v-i-ova.xml b/tests/test-v2v-i-ova.xml > index d7383905fdc0..9f3c1974243f 100644 > --- a/tests/test-v2v-i-ova.xml > +++ b/tests/test-v2v-i-ova.xml > @@ -28,7 +28,7 @@ > </disk> > <disk device='cdrom' type='file'> > <driver name='qemu' type='raw'/> > - <target dev='hda' bus='ide'/> > + <target dev='sda' bus='sata'/> > </disk> > <disk device='floppy' type='file'> > <driver name='qemu' type='raw'/> > -- > 2.19.1.3.g30247aa5d201The patch looks fine, although I think it would be better to wait until it's tested, if you can fix the librpm problem. 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
Laszlo Ersek
2021-Dec-16 10:50 UTC
[Libguestfs] [v2v PATCH v2] convert_linux: translate the first CD-ROM's references in boot conf files
On 12/15/21 16:31, Laszlo Ersek wrote:> If the only CD-ROM in "s_removables" is on an IDE controller, and the > guest kernel represents it with a /dev/hdX device node, then convert > references to this device node, in the boot config files, to /dev/cdrom. > On the destination (after conversion), /dev/cdrom will point to whataver > node we converted the CD-ROM to, masking a potential i440fx -> q35 (IDE -> > SATA) board change. > > If the only CD-ROM is not on an IDE controller, or the guest is modern > enough to represent the IDE CD-ROM as /dev/sr0, then perform no > translation. Namely, /dev/sr0 survives a potential i440fx -> q35 (IDE -> > SATA) board change intact. > > When multiple CD-ROMs exist, emit a warning, and attempt the conversion on > the first CD-ROM, as a guess. This may be inexact, but we can't do better, > because: > > - SATA, SCSI, and (on modern guests) IDE CD-ROMs are lumped together in > the /dev/sr* namespace, on the source side, and "s_removable_slot" is > useless for telling them apart, as we don't know the exact controller > topology (and OS enumeration order); > > - after conversion: some OSes don't create /dev/cdrom* symlinks to all > CD-ROMs, and even if multiple such symlinks are created, their order is > basically random. > > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1637857 > Signed-off-by: Laszlo Ersek <lersek at redhat.com> > --- > > Notes: > v2: > > - Use List.length >= 2 rather than matching with three cons operations. > [Rich] > > - At the same time, eliminate wrong precedence between "->" in a match, > and ";". [Rich] > > - Match the first element of the cdrom list, and the contents of that > element, in a single "match" expression. [Rich] > > - Not tested, due to the issue I described in > <8b3ce08b-ea47-dc1c-f441-c8b91708bd6f at redhat.com> (cannot provide a > mailing list URL because the archive seems to have stopped refreshing > itself?) > > convert/convert_linux.ml | 19 +++++++++++++++++++ > tests/test-v2v-cdrom.expected | 2 +- > tests/test-v2v-cdrom.xml.in | 4 +++- > tests/test-v2v-i-ova.xml | 2 +- > 4 files changed, 24 insertions(+), 3 deletions(-) > > diff --git a/convert/convert_linux.ml b/convert/convert_linux.ml > index 8dc648169dcb..d49ecec03aeb 100644 > --- a/convert/convert_linux.ml > +++ b/convert/convert_linux.ml > @@ -1020,6 +1020,25 @@ let convert (g : G.guestfs) source inspect keep_serial_console _ > "xvd" ^ drive_name i, block_prefix_after_conversion ^ drive_name i > ) source.s_disks in > > + (* Check the first CD-ROM. If its controller is IDE, and the OS is RHEL<=5, > + * then translate the CD-ROM from "/dev/hd[SLOT]" to "/dev/cdrom". See > + * RHBZ#1637857 for details. > + *) > + let cdroms = List.filter > + (fun removable -> removable.s_removable_type = CDROM) > + source.s_removables in > + if List.length cdroms >= 2 then > + warning (f_"multiple CD-ROMs found; translation of CD-ROM references \ > + may be inexact"); > + let map = map @ > + (match cdroms with > + | { s_removable_controller = Some Source_IDE; > + s_removable_slot = Some slot } :: _ > + when family = `RHEL_family && inspect.i_major_version <= 5 -> > + [("hd" ^ drive_name slot, "cdrom")] > + | _ -> [] > + ) in > + > if verbose () then ( > eprintf "block device map:\n"; > List.iter (After updating my workstation to F35 and rebuilding everything, this patch seems to do the right thing in a synthetic test. Namely, in the input guest, I added an entry to /etc/fstab, mounting /dev/hda (an IDE CD-ROM) as /mnt2. After conversion with virt-v2v -i libvirt -o libvirt -on rhel5.11.out rhel5.11 I checked the output disk manually with guestfish, and /dev/hda had indeed been replaced with /dev/cdrom in the same /etc/fstab entry. Additionally, when passing "-v -x" to the above command line, the debug output contains:> block device map: > hda -> cdrom > vda -> vda > xvda -> vdaSo this looks good. However... the converted guest does not boot at all. Please see the attached screenshot (it's not big). To see whether the issue was introduced by my patch, I rebuilt virt-v2v with my patch removed (that is, at current master = 1673fc4b640f), and repeated the conversion (first undefining the "rhel5.11.out" domain). The boot failure persists, so it's unrelated to the patch. Is this a known issue? Does it block merging the patch? Thanks! Laszlo> diff --git a/tests/test-v2v-cdrom.expected b/tests/test-v2v-cdrom.expected > index 34d2bf5961b0..17bd152d8e64 100644 > --- a/tests/test-v2v-cdrom.expected > +++ b/tests/test-v2v-cdrom.expected > @@ -4,5 +4,5 @@ > </disk> > <disk device='cdrom' type='file'> > <driver name='qemu' type='raw'/> > - <target dev='hdc' bus='ide'/> > + <target dev='sdc' bus='sata'/> > </disk> > diff --git a/tests/test-v2v-cdrom.xml.in b/tests/test-v2v-cdrom.xml.in > index 6bad5eab1cd4..a6e1e3f514d5 100644 > --- a/tests/test-v2v-cdrom.xml.in > +++ b/tests/test-v2v-cdrom.xml.in > @@ -35,7 +35,9 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. > <disk type='file' device='cdrom'> > <driver name='qemu' type='raw'/> > <source file='@abs_top_builddir@/test-data/phony-guests/blank-disk.img'/> > - <!-- virt-v2v should preserve the device name and bus --> > + <!-- virt-v2v should change the bus to sata, due to Windows 7 > + triggering a machine type change from i440fx to q35. Beyond that, > + virt-v2v should preserve the on-bus index. --> > <target dev='hdc' bus='ide'/> > </disk> > </devices> > diff --git a/tests/test-v2v-i-ova.xml b/tests/test-v2v-i-ova.xml > index d7383905fdc0..9f3c1974243f 100644 > --- a/tests/test-v2v-i-ova.xml > +++ b/tests/test-v2v-i-ova.xml > @@ -28,7 +28,7 @@ > </disk> > <disk device='cdrom' type='file'> > <driver name='qemu' type='raw'/> > - <target dev='hda' bus='ide'/> > + <target dev='sda' bus='sata'/> > </disk> > <disk device='floppy' type='file'> > <driver name='qemu' type='raw'/> >-------------- next part -------------- A non-text attachment was scrubbed... Name: Screenshot_rhel5.11.out_2021-12-16_11:48:24.png Type: image/png Size: 13230 bytes Desc: not available URL: <http://listman.redhat.com/archives/libguestfs/attachments/20211216/204e34f5/attachment.png>