Libguestfs - Apr 2022 - [v2v PATCH] output_qemu: rewrite output disk mapping

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

Notes:
    v1:
    
    - do not pick up Rich's R-b yet, because of the changes below
    
    - replace "parent_ctrl_needed" with Array.exists [Rich]
    
    - reference 'virt-v2v(1) section "BUGS"' in the "this
should not happen"
      warnings [Rich]
    
    - add "bus=scsi0.0" property to "scsi-hd" and
"scsi-cd" devices
    
    - created test scenarios
      <https://bugzilla.redhat.com/show_bug.cgi?id=2074805#c17> and tested
      them

 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..da8bd475e56e 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 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 +      Array.exists disk_cdrom_filter
target_buses.target_ide_bus
+    and scsi_ctrl_needed +      Array.exists disk_cdrom_filter
target_buses.target_scsi_bus
+    and floppy_ctrl_needed +      Array.exists floppy_filter
target_buses.target_floppy_bus 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 -- refer to virt-v2v(1) \
+                            section \"BUGS\".")
+    );
 
-         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 -- refer to \
+                            virt-v2v(1) section \"BUGS\".")
+    );
 
-      | 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 = [ "bus=scsi0.0";
+                           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: b3a9dd6442ea2aff31bd93e85aa130f432e24932
-- 
2.19.1.3.g30247aa5d201

On Fri, Apr 15, 2022 at 10:47:28AM +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 at redhat.com>
> ---
> 
> Notes:
>     v1:
>     
>     - do not pick up Rich's R-b yet, because of the changes below
>     
>     - replace "parent_ctrl_needed" with Array.exists [Rich]
>     
>     - reference 'virt-v2v(1) section "BUGS"' in the
"this should not happen"
>       warnings [Rich]
>     
>     - add "bus=scsi0.0" property to "scsi-hd" and
"scsi-cd" devices
>     
>     - created test scenarios
>       <https://bugzilla.redhat.com/show_bug.cgi?id=2074805#c17> and
tested
>       them
> 
>  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..da8bd475e56e 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 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 > +      Array.exists disk_cdrom_filter
target_buses.target_ide_bus
> +    and scsi_ctrl_needed > +      Array.exists disk_cdrom_filter
target_buses.target_scsi_bus
> +    and floppy_ctrl_needed > +      Array.exists floppy_filter
target_buses.target_floppy_bus 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 -- refer to
virt-v2v(1) \
> +                            section \"BUGS\".")
> +    );
>  
> -         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 -- refer to \
> +                            virt-v2v(1) section \"BUGS\".")
> +    );
>  
> -      | 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 = [ "bus=scsi0.0";
> +                           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: b3a9dd6442ea2aff31bd93e85aa130f432e24932
A style point: When defining things which are functions (even if they
are curried) I usually put the "in" on the next line as a small
reminder that it's a function and not a value.  eg:

    let disk_cdrom_filter       function
      | BusSlotDisk _
      | BusSlotRemovable { s_removable_type = CDROM } -> true
      | _ -> false
    and floppy_filter       function
      | BusSlotRemovable { s_removable_type = Floppy } -> true
      | _ -> false
    in                   <-- here

There are a few places in the code that don't follow that style.

Reviewed-by: Richard W.M. Jones <rjones at redhat.com>

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