[Libguestfs] [v2v PATCH v2] convert_linux: translate the first CD-ROM's references in boot conf files

[Richard W.M. Jones]

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.g30247aa5d201

The 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