10:07 a.m.
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(a)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
+ (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
--
2.19.1.3.g30247aa5d201
10:29 a.m.
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(a)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
10:40 a.m.
On Thu, Apr 14, 2022 at 05:07:29PM +0200, 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(a)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
You can Array.exists here, but it might also be worth converting
everything to a list earlier on in the file, eg:
let target_virtio_blk_bus = Array.to_list target_buses.target_virtio_blk_bus
and target_ide_bus = Array.to_list target_buses.target_ide_bus
[etc]
+ 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.")
So it's good that it's a warning (dropping a floppy or CD-ROM when
converting is not a reason to fail), however it could be a bit more
actionable. I would include a reference to the manual, "BUGS" section
in virt-v2v(1).
+ );
- 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.")
Same here.
+ );
- | 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
If you just need a source of unique numbers you can use “unique ()”
(from Std_utils). A possible issue with this is that you won't get
the same output on every run, especially if something unrelated inside
virt-v2v changes, but that probably doesn't matter here. Especially
since we are printing the process ID in the output already, so that
ship has sailed.
This means you wouldn't need to call Array.iteri below.
+ 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
I remember master/slave IDE ...
+ | 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
+ (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;
It's definitely not as complicated as I thought it could be.
Reviewed-by: Richard W.M. Jones <rjones(a)redhat.com>
Thanks,
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
10:43 a.m.
On Thu, Apr 14, 2022 at 04:40:25PM +0100, Richard W.M. Jones wrote:
> + (* 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
You can Array.exists here, but it might also be worth converting
^ use
everything to a list earlier on in the file, eg:
let target_virtio_blk_bus = Array.to_list target_buses.target_virtio_blk_bus
and target_ide_bus = Array.to_list target_buses.target_ide_bus
[etc]
and then using List.exists here and List.iter{,i} below.
[warning telling people to file a bug]
So it's good that it's a warning (dropping a floppy or CD-ROM when
converting is not a reason to fail), however it could be a bit more
actionable. I would include a reference to the manual, "BUGS" section
in virt-v2v(1).
Actually I'm having second thoughts about encouraging people to file a
bug at all. If lots of people see this message and file bugs, is that
something we actually would need to fix? If yes then OK, otherwise
we'll just get more bugs on the backlog.
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
1:05 a.m.
On 04/14/22 17:43, Richard W.M. Jones wrote:
On Thu, Apr 14, 2022 at 04:40:25PM +0100, Richard W.M. Jones wrote:
>> + (* 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
>
> You can Array.exists here, but it might also be worth converting
^ use
> everything to a list earlier on in the file, eg:
>
> let target_virtio_blk_bus = Array.to_list target_buses.target_virtio_blk_bus
> and target_ide_bus = Array.to_list target_buses.target_ide_bus
> [etc]
and then using List.exists here and List.iter{,i} below.
[warning telling people to file a bug]
>
> So it's good that it's a warning (dropping a floppy or CD-ROM when
> converting is not a reason to fail), however it could be a bit more
> actionable. I would include a reference to the manual, "BUGS" section
> in virt-v2v(1).
Actually I'm having second thoughts about encouraging people to file a
bug at all. If lots of people see this message and file bugs, is that
something we actually would need to fix? If yes then OK, otherwise
we'll just get more bugs on the backlog.
If those branches ever fire, then we do have a bug: it means that the
target bus assignment in "convert/target_bus_assignment.ml" created such
a machine config that is impossible to map to QEMU.
The first branch in particular could refer to an IDE *disk* (not just
CD-ROM) on the Virt machine type, which would absolutely be a bug
*somewhere*. It would mean that we found some fixed disk on the source,
and that the guest OS (aarch64 Linux, most likely) does not have a
virtio-blk-driver -- and so we'd map the disk to IDE. That mapping is
wrong for us to do; but it's even worse for the guest not to have a
virtio-blk driver!
I figured an assertion failure would be too strong for this. And yes, if
this ever pops up in practice, I think we should discuss such cases on
the list, probably telling users "your guest kernel config is broken".
Thanks,
Laszlo
2:29 a.m.
On 04/14/22 17:43, Richard W.M. Jones wrote:
On Thu, Apr 14, 2022 at 04:40:25PM +0100, Richard W.M. Jones wrote:
>> + (* 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
>
> You can Array.exists here, but it might also be worth converting
^ use
> everything to a list earlier on in the file, eg:
>
> let target_virtio_blk_bus = Array.to_list target_buses.target_virtio_blk_bus
> and target_ide_bus = Array.to_list target_buses.target_ide_bus
> [etc]
and then using List.exists here and List.iter{,i} below.
Turns out that using Array.exists is less work.
Laszlo
2:26 a.m.
On 04/14/22 17:40, Richard W.M. Jones wrote:
On Thu, Apr 14, 2022 at 05:07:29PM +0200, Laszlo Ersek wrote:
> + 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
If you just need a source of unique numbers you can use “unique ()”
(from Std_utils). A possible issue with this is that you won't get
the same output on every run, especially if something unrelated inside
virt-v2v changes, but that probably doesn't matter here. Especially
since we are printing the process ID in the output already, so that
ship has sailed.
This means you wouldn't need to call Array.iteri below.
I prefer sticking with Array.iteri: "frontend_ctr" is used in various
front-end properties, such as the primary/secondary master/slave
placement on (i440fx) IDE, the (q35) SATA port selection, the LUNs for
scsi-cd / scsi-hd, the unit number for floppies; so "frontend_ctr"
should iterate over consecutive, and small, integers.
Thanks!
Laszlo
833
days inactive
834
days old
6 comments
2 participants
participants (2)
-
Laszlo Ersek -
Richard W.M. Jones