[Libguestfs] [virt-v2v RFC] output_qemu: rewrite output disk mapping
Laszlo Ersek
lersek at redhat.com
Thu Apr 14 15:29:29 UTC 2022
On 04/14/22 17:07, Laszlo Ersek wrote:
> The current code handles some nonexistent cases (such as SCSI floppies,
> virtio-block CD-ROMs), and does not create proper drives (that is,
> back-ends with no media inserted) for removable devices (floppies,
> CD-ROMs).
>
> Rewrite the whole logic:
>
> - handle only those scenarios that QEMU can support,
>
> - separate the back-end (-drive) and front-end (-device) options,
>
> - wherever / whenever a host-bus adapter front-end is needed
> (virtio-scsi-pci, isa-fdc), create it,
>
> - assign front-ends to buses (= parent front-ends) and back-ends
> explicitly.
>
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2074805
> Signed-off-by: Laszlo Ersek <lersek at redhat.com>
> ---
> output/output_qemu.ml | 197 ++++++++++++++++----
> 1 file changed, 160 insertions(+), 37 deletions(-)
>
> diff --git a/output/output_qemu.ml b/output/output_qemu.ml
> index da7278b88b67..a25b49d86679 100644
> --- a/output/output_qemu.ml
> +++ b/output/output_qemu.ml
> @@ -127,7 +127,7 @@ module QEMU = struct
> machine, false in
> let smm = secure_boot_required in
>
> - let machine =
> + let machine_str =
> match machine with
> | I440FX -> "pc"
> | Q35 -> "q35"
> @@ -153,7 +153,7 @@ module QEMU = struct
> arg_list "-device" ["vmgenid"; sprintf "guid=%s" genid; "id=vmgenid0"]
> );
>
> - arg_list "-machine" (machine ::
> + arg_list "-machine" (machine_str ::
> (if smm then ["smm=on"] else []) @
> ["accel=kvm:tcg"]);
>
> @@ -184,52 +184,175 @@ module QEMU = struct
> );
> );
>
> - let make_disk if_name i = function
> - | BusSlotEmpty -> ()
> + (* For IDE disks, IDE CD-ROMs, SCSI disks, SCSI CD-ROMs, and floppies, we
> + * need host-bus adapters (HBAs) between these devices and the PCI(e) root
> + * bus. Some machine types create these HBAs automatically (despite
> + * "-no-user-config -nodefaults"), some don't...
> + *)
> + let parent_ctrl_needed target_bus dev_filter =
> + try ignore (List.find dev_filter (Array.to_list target_bus)); true
> + with Not_found -> false
> + and disk_cdrom_filter =
> + function
> + | BusSlotDisk _
> + | BusSlotRemovable { s_removable_type = CDROM } -> true
> + | _ -> false
> + and floppy_filter =
> + function
> + | BusSlotRemovable { s_removable_type = Floppy } -> true
> + | _ -> false in
> + let ide_ctrl_needed =
> + parent_ctrl_needed target_buses.target_ide_bus disk_cdrom_filter
> + and scsi_ctrl_needed =
> + parent_ctrl_needed target_buses.target_scsi_bus disk_cdrom_filter
> + and floppy_ctrl_needed =
> + parent_ctrl_needed target_buses.target_floppy_bus floppy_filter in
>
> - | BusSlotDisk d ->
> - (* Find the corresponding target disk. *)
> - let outdisk = disk_path output_storage output_name d.s_disk_id in
> + if ide_ctrl_needed then (
> + match machine with
> + | I440FX -> ()
> + (* The PC machine has a built-in controller of type "piix3-ide"
> + * providing buses "ide.0" and "ide.1", with each bus fitting two
> + * devices.
> + *)
> + | Q35 -> ()
> + (* The Q35 machine has a built-in controller of type "ich9-ahci"
> + * providing buses "ide.0" through "ide.5", with each bus fitting one
> + * device.
> + *)
> + | Virt -> warning (f_"The Virt machine has no support for IDE. Please \
> + report a bug for virt-v2v.")
> + );
>
> - arg_list "-drive" ["file=" ^ outdisk; "format=" ^ output_format;
> - "if=" ^ if_name; "index=" ^ string_of_int i;
> - "media=disk"]
> + if scsi_ctrl_needed then
> + (* We need to add the virtio-scsi HBA on all three machine types. The bus
> + * provided by this device will be called "scsi0.0".
> + *)
> + arg_list "-device" [ "virtio-scsi-pci"; "id=scsi0" ];
>
> - | BusSlotRemovable { s_removable_type = CDROM } ->
> - arg_list "-drive" ["format=raw"; "if=" ^ if_name;
> - "index=" ^ string_of_int i; "media=cdrom"]
> + if floppy_ctrl_needed then (
> + match machine with
> + | I440FX -> ()
> + (* The PC machine has a built-in controller of type "isa-fdc"
> + * providing bus "floppy-bus.0", fitting two devices.
> + *)
> + | Q35 -> arg_list "-device" [ "isa-fdc"; "id=floppy-bus" ]
> + (* On the Q35 machine, we need to add the same HBA manually. Note that
> + * the bus name will have ".0" appended automatically.
> + *)
> + | Virt -> warning (f_"The Virt machine has no support for floppies. \
> + Please report a bug for virt-v2v.")
> + );
>
> - | BusSlotRemovable { s_removable_type = Floppy } ->
> - arg_list "-drive" ["format=raw"; "if=" ^ if_name;
> - "index=" ^ string_of_int i; "media=floppy"]
> - in
> - Array.iteri (make_disk "virtio") target_buses.target_virtio_blk_bus;
> - Array.iteri (make_disk "ide") target_buses.target_ide_bus;
> + let add_disk_backend disk_id backend_name =
> + (* Add a drive (back-end) for a "virtio-blk-pci", "ide-hd", or "scsi-hd"
> + * device (front-end). The drive has a backing file, identified by
> + * "disk_id".
> + *)
> + let outdisk = disk_path output_storage output_name disk_id in
> + arg_list "-drive" [ "file=" ^ outdisk; "format=" ^ output_format;
> + "if=none"; "id=" ^ backend_name; "media=disk" ]
>
> - let make_scsi i = function
> - | BusSlotEmpty -> ()
> + and add_cdrom_backend backend_name =
> + (* Add a drive (back-end) for an "ide-cd" or "scsi-cd" device (front-end).
> + * The drive is empty -- there is no backing file.
> + *)
> + arg_list "-drive" [ "if=none"; "id=" ^ backend_name; "media=cdrom" ]
>
> - | BusSlotDisk d ->
> - (* Find the corresponding target disk. *)
> - let outdisk = disk_path output_storage output_name d.s_disk_id in
> + and add_floppy_backend backend_name =
> + (* Add a drive (back-end) for a "floppy" device (front-end). The drive is
> + * empty -- there is no backing file. *)
> + arg_list "-drive" [ "if=none"; "id=" ^ backend_name; "media=disk" ] in
>
> - arg_list "-drive" ["file=" ^ outdisk; "format=" ^ output_format;
> - "if=scsi"; "bus=0"; "unit=" ^ string_of_int i;
> - "media=disk"]
> + let add_virtio_blk disk_id frontend_ctr =
> + (* Create a "virtio-blk-pci" device (front-end), together with its drive
> + * (back-end). The disk identifier is mandatory.
> + *)
> + let backend_name = sprintf "drive-vblk-%d" frontend_ctr in
> + add_disk_backend disk_id backend_name;
> + arg_list "-device" [ "virtio-blk-pci"; "drive=" ^ backend_name ]
>
> - | BusSlotRemovable { s_removable_type = CDROM } ->
> - arg_list "-drive" ["format=raw"; "if=scsi"; "bus=0";
> - "unit=" ^ string_of_int i; "media=cdrom"]
> + and add_ide disk_id frontend_ctr =
> + (* Create an "ide-hd" or "ide-cd" device (front-end), together with its
> + * drive (back-end). If a disk identifier is passed in, then "ide-hd" is
> + * created (with a non-empty drive); otherwise, "ide-cd" is created (with
> + * an empty drive).
> + *)
> + let backend_name = sprintf "drive-ide-%d" frontend_ctr
> + and ide_bus, ide_unit =
> + match machine with
> + | I440FX -> frontend_ctr / 2, frontend_ctr mod 2
> + | Q35 -> frontend_ctr, 0
> + | Virt -> 0, 0 (* should never happen, see warning above *) in
> + let common_props = [ sprintf "bus=ide.%d" ide_bus;
> + sprintf "unit=%d" ide_unit;
> + "drive=" ^ backend_name ] in
> + (match disk_id with
> + | Some id ->
> + add_disk_backend id backend_name;
> + arg_list "-device" ([ "ide-hd" ] @ common_props)
> + | None ->
> + add_cdrom_backend backend_name;
> + arg_list "-device" ([ "ide-cd" ] @ common_props))
> +
> + and add_scsi disk_id frontend_ctr =
> + (* Create a "scsi-hd" or "scsi-cd" device (front-end), together with its
> + * drive (back-end). If a disk identifier is passed in, then "scsi-hd" is
> + * created (with a non-empty drive); otherwise, "scsi-cd" is created (with
> + * an empty drive).
> + *)
> + let backend_name = sprintf "drive-scsi-%d" frontend_ctr in
> + let common_props = [ sprintf "lun=%d" frontend_ctr;
> + "drive=" ^ backend_name ] in
... since posting, I've prepended "bus=scsi0.0" here.
Thanks
Laszlo
> + (match disk_id with
> + | Some id ->
> + add_disk_backend id backend_name;
> + arg_list "-device" ([ "scsi-hd" ] @ common_props)
> + | None ->
> + add_cdrom_backend backend_name;
> + arg_list "-device" ([ "scsi-cd" ] @ common_props))
>
> - | BusSlotRemovable { s_removable_type = Floppy } ->
> - arg_list "-drive" ["format=raw"; "if=scsi"; "bus=0";
> - "unit=" ^ string_of_int i; "media=floppy"]
> - in
> - Array.iteri make_scsi target_buses.target_scsi_bus;
> + and add_floppy frontend_ctr =
> + (* Create a "floppy" (front-end), together with its empty drive
> + * (back-end).
> + *)
> + let backend_name = sprintf "drive-floppy-%d" frontend_ctr in
> + add_floppy_backend backend_name;
> + arg_list "-device" [ "floppy"; "bus=floppy-bus.0";
> + sprintf "unit=%d" frontend_ctr;
> + "drive=" ^ backend_name ] in
>
> - (* XXX Highly unlikely that anyone cares, but the current
> - * code ignores target_buses.target_floppy_bus.
> + (* Add virtio-blk-pci devices for BusSlotDisk elements on
> + * "target_virtio_blk_bus".
> *)
> + Array.iteri
> + (fun frontend_ctr disk ->
> + match disk with
> + | BusSlotDisk d -> add_virtio_blk d.s_disk_id frontend_ctr
> + | _ -> ())
> + target_buses.target_virtio_blk_bus;
> +
> + let add_disk_or_cdrom bus_adder frontend_ctr slot =
> + (* Add a disk or CD-ROM front-end to the IDE or SCSI bus. *)
> + match slot with
> + | BusSlotDisk d ->
> + bus_adder (Some d.s_disk_id) frontend_ctr
> + | BusSlotRemovable { s_removable_type = CDROM } ->
> + bus_adder None frontend_ctr
> + | _ -> () in
> +
> + (* Add disks and CD-ROMs to the IDE and SCSI buses. *)
> + Array.iteri (add_disk_or_cdrom add_ide) target_buses.target_ide_bus;
> + Array.iteri (add_disk_or_cdrom add_scsi) target_buses.target_scsi_bus;
> +
> + (* Add floppies. *)
> + Array.iteri
> + (fun frontend_ctr disk ->
> + match disk with
> + | BusSlotRemovable { s_removable_type = Floppy } ->
> + add_floppy frontend_ctr
> + | _ -> ())
> + target_buses.target_floppy_bus;
>
> let net_bus =
> match guestcaps.gcaps_net_bus with
>
> base-commit: e3226dd431e7476cae2d0ca54b2332344de2d201
>
More information about the Libguestfs
mailing list