Libguestfs - Jun 2016 - [PATCH 1/2] v2v: Fix conversion of floppy removable devices (RHBZ#1309706).

The previous code treated floppy disks and CD-ROMs as the same kind of
thing, resulting in malformed libvirt XML.  You would see the
following error when importing a guest into libvirt:

  error: Failed to define domain from /tmp/v2vlibvirt063486.xml
  error: internal error: Invalid floppy device name: hdb

because we incorrectly generated this bogus libvirt XML fragment:

  <disk device='floppy' type='file'>
    <driver name='qemu' type='raw'/>
    <target dev='hdb' bus='ide'/>
  </disk>

This commit models floppy devices as a distinct type, occupying their
own bus ("/dev/fdX").  When writing to libvirt, we generate correct
XML fragments, looking like this:

  <disk device='floppy' type='file'>
    <driver name='qemu' type='raw'/>
    <target dev='fda'/>
  </disk>

Miscellaneous other changes were required in the code.  There is also
a regression test (see following commit).

Note this ignores floppy disks in '-o qemu' mode.
---
 v2v/input_libvirtxml.ml      |  1 +
 v2v/output_libvirt.ml        |  6 ++++--
 v2v/output_qemu.ml           |  4 ++++
 v2v/target_bus_assignment.ml | 20 ++++++++++++--------
 v2v/test-v2v-i-ova.xml       |  2 +-
 v2v/types.ml                 |  1 +
 v2v/types.mli                | 10 +++++++---
 7 files changed, 30 insertions(+), 14 deletions(-)

diff --git a/v2v/input_libvirtxml.ml b/v2v/input_libvirtxml.ml
index 24e3b74..552bd9f 100644
--- a/v2v/input_libvirtxml.ml
+++ b/v2v/input_libvirtxml.ml
@@ -319,6 +319,7 @@ let parse_libvirt_xml ?conn xml          | Some s when
String.is_prefix s "sd" -> get_drive_slot s 2
         | Some s when String.is_prefix s "vd" -> get_drive_slot s
2
         | Some s when String.is_prefix s "xvd" -> get_drive_slot s
3
+        | Some s when String.is_prefix s "fd" -> get_drive_slot s
2
         | Some s ->
            warning (f_"<target dev='%s'> was ignored because
the device name could not be recognized") s;
            None in
diff --git a/v2v/output_libvirt.ml b/v2v/output_libvirt.ml
index d56a852..e7067d7 100644
--- a/v2v/output_libvirt.ml
+++ b/v2v/output_libvirt.ml
@@ -171,7 +171,6 @@ let create_libvirt_xml ?pool source target_buses guestcaps
           e "driver" [ "name", "qemu";
"type", "raw" ] [];
           e "target" [
             "dev", drive_prefix ^ drive_name i;
-            "bus", bus_name
           ] []
         ]
     in
@@ -185,7 +184,10 @@ let create_libvirt_xml ?pool source target_buses guestcaps
                     target_buses.target_ide_bus);
       Array.to_list
         (Array.mapi (make_disk "scsi" "sd")
-                    target_buses.target_scsi_bus)
+                    target_buses.target_scsi_bus);
+      Array.to_list
+        (Array.mapi (make_disk "floppy" "fd")
+                    target_buses.target_floppy_bus)
     ] in
 
   let nics diff --git a/v2v/output_qemu.ml b/v2v/output_qemu.ml
index f1d3c5f..b6093be 100644
--- a/v2v/output_qemu.ml
+++ b/v2v/output_qemu.ml
@@ -137,6 +137,10 @@ object
     in
     Array.iteri make_scsi target_buses.target_scsi_bus;
 
+    (* XXX Highly unlikely that anyone cares, but the current
+     * code ignores target_buses.target_floppy_bus.
+     *)
+
     let net_bus        match guestcaps.gcaps_net_bus with
       | Virtio_net -> "virtio-net-pci"
diff --git a/v2v/target_bus_assignment.ml b/v2v/target_bus_assignment.ml
index ceb0550..d3444bc 100644
--- a/v2v/target_bus_assignment.ml
+++ b/v2v/target_bus_assignment.ml
@@ -21,11 +21,11 @@ open Common_gettext.Gettext
 
 open Types
 
-(* XXX This doesn't do the right thing for PC legacy floppy devices. *)
 let rec target_bus_assignment source targets guestcaps    let virtio_blk_bus =
ref [| |]
   and ide_bus = ref [| |]
-  and scsi_bus = ref [| |] in
+  and scsi_bus = ref [| |]
+  and floppy_bus = ref [| |] in
 
   (* Add the fixed disks (targets) to either the virtio-blk or IDE bus,
    * depending on whether the guest has virtio drivers or not.
@@ -66,11 +66,14 @@ let rec target_bus_assignment source targets guestcaps      
fun r ->
         let t = BusSlotRemovable r in
         let bus -          match r.s_removable_controller with
-          | None -> ide_bus (* Wild guess, but should be safe. *)
-          | Some Source_virtio_blk -> virtio_blk_bus
-          | Some Source_IDE -> ide_bus
-          | Some Source_virtio_SCSI | Some Source_SCSI -> scsi_bus in
+          match r.s_removable_type with
+          | CDROM ->
+             (match r.s_removable_controller with
+              | None -> ide_bus (* Wild guess, but should be safe. *)
+              | Some Source_virtio_blk -> virtio_blk_bus
+              | Some Source_IDE -> ide_bus
+              | Some Source_virtio_SCSI | Some Source_SCSI -> scsi_bus)
+          | Floppy -> floppy_bus in
 
         match r.s_removable_slot with
         | None ->
@@ -89,7 +92,8 @@ let rec target_bus_assignment source targets guestcaps  
   { target_virtio_blk_bus = !virtio_blk_bus;
     target_ide_bus = !ide_bus;
-    target_scsi_bus = !scsi_bus }
+    target_scsi_bus = !scsi_bus;
+    target_floppy_bus = !floppy_bus }
 
 (* Insert a slot into the bus array, making the array bigger if necessary. *)
 and insert bus i slot diff --git a/v2v/test-v2v-i-ova.xml
b/v2v/test-v2v-i-ova.xml
index bb765e3..6dcfc31 100644
--- a/v2v/test-v2v-i-ova.xml
+++ b/v2v/test-v2v-i-ova.xml
@@ -27,7 +27,7 @@
     </disk>
     <disk device='floppy' type='file'>
       <driver name='qemu' type='raw'/>
-      <target dev='hdb' bus='ide'/>
+      <target dev='fda'/>
     </disk>
     <interface type='network'>
       <source network='Ethernet 1'/>
diff --git a/v2v/types.ml b/v2v/types.ml
index bff03a2..c43db24 100644
--- a/v2v/types.ml
+++ b/v2v/types.ml
@@ -425,6 +425,7 @@ type target_buses = {
   target_virtio_blk_bus : target_bus_slot array;
   target_ide_bus : target_bus_slot array;
   target_scsi_bus : target_bus_slot array;
+  target_floppy_bus : target_bus_slot array;
 }
 
 and target_bus_slot diff --git a/v2v/types.mli b/v2v/types.mli
index 25420df..2727383 100644
--- a/v2v/types.mli
+++ b/v2v/types.mli
@@ -278,10 +278,11 @@ type target_buses = {
   target_virtio_blk_bus : target_bus_slot array;
   target_ide_bus : target_bus_slot array;
   target_scsi_bus : target_bus_slot array;
+  target_floppy_bus : target_bus_slot array;
 }
 (** Mapping of fixed and removable disks to buses.
 
-    As shown in the diagram below, there are (currently) three buses
+    As shown in the diagram below, there are (currently) four buses
     attached to the target VM.  Each contains a chain of fixed or
     removable disks.  Slots can also be empty.
 
@@ -300,8 +301,11 @@ type target_buses = {
    ├────┤ hda ├───┤ hdb ├───┤ hdc ├───┤ hdd │  IDE bus
    │    └─────┘   └─────┘   └─────┘   └─────┘
    │    ┌─────┐   ┌─────┐
-   └────┤  -  ├───┤ vdb │  Virtio-blk bus
-        └─────┘   └─────┘
+   ├────┤  -  ├───┤ vdb │  Virtio-blk bus
+   │    └─────┘   └─────┘
+   │    ┌─────┐
+   └────┤ fda │  Floppy disks
+        └─────┘
 v}
  *)
 
-- 
2.7.4

---
 v2v/Makefile.am              |  4 +++
 v2v/test-v2v-floppy.expected |  8 +++++
 v2v/test-v2v-floppy.sh       | 77 ++++++++++++++++++++++++++++++++++++++++++++
 v2v/test-v2v-floppy.xml      | 42 ++++++++++++++++++++++++
 4 files changed, 131 insertions(+)
 create mode 100644 v2v/test-v2v-floppy.expected
 create mode 100755 v2v/test-v2v-floppy.sh
 create mode 100644 v2v/test-v2v-floppy.xml

diff --git a/v2v/Makefile.am b/v2v/Makefile.am
index 4ae0ebb..6739a3c 100644
--- a/v2v/Makefile.am
+++ b/v2v/Makefile.am
@@ -289,6 +289,7 @@ endif
 if ENABLE_APPLIANCE
 TESTS += \
 	test-v2v-cdrom.sh \
+	test-v2v-floppy.sh \
 	test-v2v-i-ova.sh \
 	test-v2v-i-disk.sh \
 	test-v2v-in-place.sh \
@@ -352,6 +353,9 @@ EXTRA_DIST += \
 	test-v2v-cdrom.sh \
 	test-v2v-cdrom.xml \
 	test-v2v-copy-to-local.sh \
+	test-v2v-floppy.expected \
+	test-v2v-floppy.sh \
+	test-v2v-floppy.xml \
 	test-v2v-i-disk.sh \
 	test-v2v-i-ova-formats.expected \
 	test-v2v-i-ova-formats.ovf \
diff --git a/v2v/test-v2v-floppy.expected b/v2v/test-v2v-floppy.expected
new file mode 100644
index 0000000..dd74ed9
--- /dev/null
+++ b/v2v/test-v2v-floppy.expected
@@ -0,0 +1,8 @@
+    <disk type='file' device='disk'>
+      <driver name='qemu' type='raw'
cache='none'/>
+      <target dev='vda' bus='virtio'/>
+    </disk>
+    <disk device='floppy' type='file'>
+      <driver name='qemu' type='raw'/>
+      <target dev='fda'/>
+    </disk>
diff --git a/v2v/test-v2v-floppy.sh b/v2v/test-v2v-floppy.sh
new file mode 100755
index 0000000..d4ecfba
--- /dev/null
+++ b/v2v/test-v2v-floppy.sh
@@ -0,0 +1,77 @@
+#!/bin/bash -
+# libguestfs virt-v2v test script
+# Copyright (C) 2015-2016 Red Hat Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+# Test converting a guest with a floppy disk.
+# https://bugzilla.redhat.com/show_bug.cgi?id=1309706
+
+unset CDPATH
+export LANG=C
+set -e
+
+if [ -n "$SKIP_TEST_V2V_FLOPPY_SH" ]; then
+    echo "$0: test skipped because environment variable is set"
+    exit 77
+fi
+
+if [ "$(guestfish get-backend)" = "uml" ]; then
+    echo "$0: test skipped because UML backend does not support
network"
+    exit 77
+fi
+
+abs_builddir="$(pwd)"
+libvirt_uri="test://$abs_builddir/test-v2v-floppy.xml"
+
+f=../test-data/phony-guests/windows.img
+if ! test -f $f || ! test -s $f; then
+    echo "$0: test skipped because phony Windows image was not
created"
+    exit 77
+fi
+
+f=../test-data/phony-guests/blank-disk.img
+if ! test -f $f || ! test -s $f; then
+    echo "$0: test skipped because blank-disk.img was not created"
+    exit 77
+fi
+
+export VIRT_TOOLS_DATA_DIR="$srcdir/../test-data/fake-virt-tools"
+export VIRTIO_WIN="$srcdir/../test-data/fake-virtio-win"
+
+d=test-v2v-floppy.d
+rm -rf $d
+mkdir $d
+
+$VG virt-v2v --debug-gc \
+    -i libvirt -ic "$libvirt_uri" windows \
+    -o local -os $d --no-copy
+
+# Test the libvirt XML metadata was created.
+test -f $d/windows.xml
+
+# Grab just the <disk>..</disk> output and compare it to what we
+# expect.  https://stackoverflow.com/questions/16587218
+awk '/<disk /{p=1;print;next}
p&&/<\/disk>/{p=0;print;next} ;p' \
+    $d/windows.xml |
+    grep -v '<source file' > $d/disks
+
+if ! diff -u test-v2v-floppy.expected $d/disks; then
+    echo "$0: unexpected disk assignments"
+    cat $d/disks
+    exit 1
+fi
+
+rm -r $d
diff --git a/v2v/test-v2v-floppy.xml b/v2v/test-v2v-floppy.xml
new file mode 100644
index 0000000..41e1bef
--- /dev/null
+++ b/v2v/test-v2v-floppy.xml
@@ -0,0 +1,42 @@
+<!--
+libguestfs virt-v2v tool
+Copyright (C) 2009-2016 Red Hat Inc.
+
+This program is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 2 of the License, or
+(at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with this program; if not, write to the Free Software
+Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+-->
+<node>
+  <domain type='test'>
+    <name>windows</name>
+    <memory>1048576</memory>
+    <os>
+      <type>hvm</type>
+      <boot dev='hd'/>
+    </os>
+    <devices>
+      <disk type='file' device='disk'>
+        <driver name='qemu' type='raw'/>
+        <source file='../test-data/phony-guests/windows.img'/>
+        <!-- virt-v2v should install virtio drivers and turn this
+             into dev='vda' bus='virtio' -->
+        <target dev='hda' bus='ide'/>
+      </disk>
+      <disk type='file' device='floppy'>
+        <driver name='qemu' type='raw'/>
+        <source file='../test-data/phony-guests/blank-disk.img'/>
+        <target dev='fda' />
+      </disk>
+    </devices>
+  </domain>
+</node>
-- 
2.7.4