thr3ads.net - Libguestfs - Re: [Libguestfs] [PATCH 1/9] v2v: Stable bus and slot numbers for removable drives (RHBZ#1238053). [Jul 2015]

If this information is useful, please help other people find it:
Share via:

Richard W.M. Jones

2015-Jul-01 17:54 UTC

[Libguestfs] [PATCH 1/9] v2v: Stable bus and slot numbers for removable drives (RHBZ#1238053).

This patch series adds stable bus and slot numbers for removable
drives (CDs and floppies) when the guest is converted using virt-v2v
or virt-p2v.

Previously we were a bit random about this.  After this patch series,
the bus and slot numbers and preserved if at all possible.

BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1238053

Rich.

Richard W.M. Jones

2015-Jul-01 17:54 UTC

head link

[Libguestfs] [PATCH 1/9] utils: Add a test of the guestfs_int_drive_name function.

This function didn't have a unit test before.
---
 src/test-utils.c | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/src/test-utils.c b/src/test-utils.c
index 8e1491f..c5e2f08 100644
--- a/src/test-utils.c
+++ b/src/test-utils.c
@@ -1,5 +1,5 @@
 /* libguestfs
- * Copyright (C) 2014 Red Hat Inc.
+ * Copyright (C) 2014-2015 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
@@ -162,6 +162,32 @@ test_validate_guid (void)
   assert (guestfs_int_validate_guid
("21EC2020-3AEA-1069-A2DD-08002B30309D") == 1);
 }
 
+/* Test guestfs_int_drive_name. */
+static void
+test_drive_name (void)
+{
+  char s[10];
+
+  guestfs_int_drive_name (0, s);
+  assert (STREQ (s, "a"));
+  guestfs_int_drive_name (25, s);
+  assert (STREQ (s, "z"));
+  guestfs_int_drive_name (26, s);
+  assert (STREQ (s, "aa"));
+  guestfs_int_drive_name (27, s);
+  assert (STREQ (s, "ab"));
+  guestfs_int_drive_name (51, s);
+  assert (STREQ (s, "az"));
+  guestfs_int_drive_name (52, s);
+  assert (STREQ (s, "ba"));
+  guestfs_int_drive_name (701, s);
+  assert (STREQ (s, "zz"));
+  guestfs_int_drive_name (702, s);
+  assert (STREQ (s, "aaa"));
+  guestfs_int_drive_name (18277, s);
+  assert (STREQ (s, "zzz"));
+}
+
 int
 main (int argc, char *argv[])
 {
@@ -169,6 +195,7 @@ main (int argc, char *argv[])
   test_concat ();
   test_join ();
   test_validate_guid ();
+  test_drive_name ();
 
   exit (EXIT_SUCCESS);
 }
-- 
2.3.1

Richard W.M. Jones

2015-Jul-01 17:54 UTC

head link

[Libguestfs] [PATCH 2/9] utils: Add guestfs_int_drive_index and unit test.

Add guestfs_int_drive_index which does basically the opposite of
guestfs_int_drive_name.  This commit also includes a unit test.
---
 src/guestfs-internal-frontend.h |  1 +
 src/test-utils.c                | 22 ++++++++++++++++++++++
 src/utils.c                     | 21 +++++++++++++++++++++
 3 files changed, 44 insertions(+)

diff --git a/src/guestfs-internal-frontend.h b/src/guestfs-internal-frontend.h
index 9322201..e3f0db6 100644
--- a/src/guestfs-internal-frontend.h
+++ b/src/guestfs-internal-frontend.h
@@ -105,6 +105,7 @@ extern char **guestfs_int_split_string (char sep, const char
*);
 extern char *guestfs_int_exit_status_to_string (int status, const char
*cmd_name, char *buffer, size_t buflen);
 extern int guestfs_int_random_string (char *ret, size_t len);
 extern char *guestfs_int_drive_name (size_t index, char *ret);
+extern ssize_t guestfs_int_drive_index (const char *);
 extern int guestfs_int_is_true (const char *str);
 
 /* These functions are used internally by the CLEANUP_* macros.
diff --git a/src/test-utils.c b/src/test-utils.c
index c5e2f08..0b7714a 100644
--- a/src/test-utils.c
+++ b/src/test-utils.c
@@ -188,6 +188,27 @@ test_drive_name (void)
   assert (STREQ (s, "zzz"));
 }
 
+/* Test guestfs_int_drive_index. */
+static void
+test_drive_index (void)
+{
+  assert (guestfs_int_drive_index ("a") == 0);
+  assert (guestfs_int_drive_index ("z") == 25);
+  assert (guestfs_int_drive_index ("aa") == 26);
+  assert (guestfs_int_drive_index ("ab") == 27);
+  assert (guestfs_int_drive_index ("az") == 51);
+  assert (guestfs_int_drive_index ("ba") == 52);
+  assert (guestfs_int_drive_index ("zz") == 701);
+  assert (guestfs_int_drive_index ("aaa") == 702);
+  assert (guestfs_int_drive_index ("zzz") == 18277);
+
+  assert (guestfs_int_drive_index ("") == -1);
+  assert (guestfs_int_drive_index ("abc123") == -1);
+  assert (guestfs_int_drive_index ("123") == -1);
+  assert (guestfs_int_drive_index ("Z") == -1);
+  assert (guestfs_int_drive_index ("aB") == -1);
+}
+
 int
 main (int argc, char *argv[])
 {
@@ -196,6 +217,7 @@ main (int argc, char *argv[])
   test_join ();
   test_validate_guid ();
   test_drive_name ();
+  test_drive_index ();
 
   exit (EXIT_SUCCESS);
 }
diff --git a/src/utils.c b/src/utils.c
index 7cee16e..e9ec38e 100644
--- a/src/utils.c
+++ b/src/utils.c
@@ -271,6 +271,27 @@ guestfs_int_drive_name (size_t index, char *ret)
   return ret;
 }
 
+/* The opposite of guestfs_int_drive_name.  Take a string like "ab"
+ * and return the index (eg 27).  Note that you must remove any prefix
+ * such as "hd", "sd" etc, or any partition number before
calling the
+ * function.
+ */
+ssize_t
+guestfs_int_drive_index (const char *name)
+{
+  ssize_t r = 0;
+
+  while (*name) {
+    if (*name >= 'a' && *name <= 'z')
+      r = 26*r + (*name - 'a' + 1);
+    else
+      return -1;
+    name++;
+  }
+
+  return r-1;
+}
+
 /* Similar to Tcl_GetBoolean. */
 int
 guestfs_int_is_true (const char *str)
-- 
2.3.1

Richard W.M. Jones

2015-Jul-01 17:54 UTC

head link

[Libguestfs] [PATCH 3/9] v2v: Refactor ~printer parameter in unit tests.

Simple refactoring, no functional change.
---
 v2v/v2v_unit_tests.ml | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/v2v/v2v_unit_tests.ml b/v2v/v2v_unit_tests.ml
index 5cfb99a..aa27b9f 100644
--- a/v2v/v2v_unit_tests.ml
+++ b/v2v/v2v_unit_tests.ml
@@ -29,52 +29,53 @@ let test_get_ostype ctx              i_root = "";
i_package_format = ""; i_package_management = "";
             i_product_name = ""; i_product_variant = "";
i_mountpoints = [];
             i_apps = []; i_apps_map = StringMap.empty; i_uefi = false } in
-  assert_equal ~printer:identity "RHEL6"
+  let printer = identity in
+  assert_equal ~printer "RHEL6"
                (OVF.get_ostype { i with i_type = "linux"; i_distro =
"rhel";
                                         i_major_version = 6;
                                         i_minor_version = 0;
                                         i_arch = "i386" });
-  assert_equal ~printer:identity "RHEL6x64"
+  assert_equal ~printer "RHEL6x64"
                (OVF.get_ostype { i with i_type = "linux"; i_distro =
"rhel";
                                         i_major_version = 6;
                                         i_minor_version = 0;
                                         i_arch = "x86_64" });
-  assert_equal ~printer:identity "rhel_7x64"
+  assert_equal ~printer "rhel_7x64"
                (OVF.get_ostype { i with i_type = "linux"; i_distro =
"rhel";
                                         i_major_version = 7;
                                         i_minor_version = 0;
                                         i_arch = "x86_64" });
-  assert_equal ~printer:identity "Windows7"
+  assert_equal ~printer "Windows7"
                (OVF.get_ostype { i with i_type = "windows";
                                         i_major_version = 6;
                                         i_minor_version = 1;
                                         i_product_variant = "Client";
                                         i_arch = "i386" });
-  assert_equal ~printer:identity "Windows7x64"
+  assert_equal ~printer "Windows7x64"
                (OVF.get_ostype { i with i_type = "windows";
                                         i_major_version = 6;
                                         i_minor_version = 1;
                                         i_product_variant = "Client";
                                         i_arch = "x86_64" });
-  assert_equal ~printer:identity "windows_8"
+  assert_equal ~printer "windows_8"
                (OVF.get_ostype { i with i_type = "windows";
                                         i_major_version = 6;
                                         i_minor_version = 2;
                                         i_product_variant = "Client";
                                         i_arch = "i386" });
-  assert_equal ~printer:identity "windows_8x64"
+  assert_equal ~printer "windows_8x64"
                (OVF.get_ostype { i with i_type = "windows";
                                         i_major_version = 6;
                                         i_minor_version = 2;
                                         i_product_variant = "Client";
                                         i_arch = "x86_64" });
-  assert_equal ~printer:identity "windows_2012x64"
+  assert_equal ~printer "windows_2012x64"
                (OVF.get_ostype { i with i_type = "windows";
                                         i_major_version = 6;
                                         i_minor_version = 2;
                                         i_product_variant = "Server";
                                         i_arch = "x86_64" });
-  assert_equal ~printer:identity "windows_2012R2x64"
+  assert_equal ~printer "windows_2012R2x64"
                (OVF.get_ostype { i with i_type = "windows";
                                         i_major_version = 6;
                                         i_minor_version = 3;
-- 
2.3.1

Richard W.M. Jones

2015-Jul-01 17:54 UTC

head link

[Libguestfs] [PATCH 4/9] v2v: Add unit test of Utils.drive_name function.

---
 v2v/v2v_unit_tests.ml | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/v2v/v2v_unit_tests.ml b/v2v/v2v_unit_tests.ml
index aa27b9f..1b84ed0 100644
--- a/v2v/v2v_unit_tests.ml
+++ b/v2v/v2v_unit_tests.ml
@@ -82,11 +82,24 @@ let test_get_ostype ctx                                     
i_product_variant = "Server";
                                         i_arch = "x86_64" })
 
+let test_drive_name ctx +  let printer = identity in
+  assert_equal ~printer "a" (Utils.drive_name 0);
+  assert_equal ~printer "z" (Utils.drive_name 25);
+  assert_equal ~printer "aa" (Utils.drive_name 26);
+  assert_equal ~printer "ab" (Utils.drive_name 27);
+  assert_equal ~printer "az" (Utils.drive_name 51);
+  assert_equal ~printer "ba" (Utils.drive_name 52);
+  assert_equal ~printer "zz" (Utils.drive_name 701);
+  assert_equal ~printer "aaa" (Utils.drive_name 702);
+  assert_equal ~printer "zzz" (Utils.drive_name 18277)
+
 (* Suites declaration. *)
 let suite    "virt-v2v" >:::
     [
       "OVF.get_ostype" >:: test_get_ostype;
+      "Utils.drive_name" >:: test_drive_name;
     ]
 
 let () -- 
2.3.1

Richard W.M. Jones

2015-Jul-01 17:54 UTC

head link

[Libguestfs] [PATCH 5/9] v2v: Add a binding for guestfs_int_drive_index and a unit test.

---
 v2v/utils-c.c         | 14 ++++++++++++++
 v2v/utils.ml          |  1 +
 v2v/v2v_unit_tests.ml | 19 +++++++++++++++++++
 3 files changed, 34 insertions(+)

diff --git a/v2v/utils-c.c b/v2v/utils-c.c
index e88620f..c0fa260 100644
--- a/v2v/utils-c.c
+++ b/v2v/utils-c.c
@@ -23,6 +23,7 @@
 #include <unistd.h>
 
 #include <caml/alloc.h>
+#include <caml/fail.h>
 #include <caml/memory.h>
 #include <caml/mlvalues.h>
 
@@ -43,3 +44,16 @@ v2v_utils_drive_name (value indexv)
 
   CAMLreturn (namev);
 }
+
+value
+v2v_utils_drive_index (value strv)
+{
+  CAMLparam1 (strv);
+  ssize_t r;
+
+  r = guestfs_int_drive_index (String_val (strv));
+  if (r == -1)
+    caml_invalid_argument ("drive_index: invalid parameter");
+
+  CAMLreturn (Val_int (r));
+}
diff --git a/v2v/utils.ml b/v2v/utils.ml
index 2aeba45..5618026 100644
--- a/v2v/utils.ml
+++ b/v2v/utils.ml
@@ -59,6 +59,7 @@ let uri_quote str    String.concat "" (List.rev !xs)
 
 external drive_name : int -> string = "v2v_utils_drive_name"
+external drive_index : string -> int = "v2v_utils_drive_index"
 
 (* Map guest architecture found by inspection to the architecture
  * that KVM must emulate.  Note for x86 we assume a 64 bit hypervisor.
diff --git a/v2v/v2v_unit_tests.ml b/v2v/v2v_unit_tests.ml
index 1b84ed0..00d19e0 100644
--- a/v2v/v2v_unit_tests.ml
+++ b/v2v/v2v_unit_tests.ml
@@ -94,12 +94,31 @@ let test_drive_name ctx    assert_equal ~printer
"aaa" (Utils.drive_name 702);
   assert_equal ~printer "zzz" (Utils.drive_name 18277)
 
+let test_drive_index ctx +  let printer = string_of_int in
+  assert_equal ~printer 0 (Utils.drive_index "a");
+  assert_equal ~printer 25 (Utils.drive_index "z");
+  assert_equal ~printer 26 (Utils.drive_index "aa");
+  assert_equal ~printer 27 (Utils.drive_index "ab");
+  assert_equal ~printer 51 (Utils.drive_index "az");
+  assert_equal ~printer 52 (Utils.drive_index "ba");
+  assert_equal ~printer 701 (Utils.drive_index "zz");
+  assert_equal ~printer 702 (Utils.drive_index "aaa");
+  assert_equal ~printer 18277 (Utils.drive_index "zzz");
+  let exn = Invalid_argument "drive_index: invalid parameter" in
+  assert_raises exn (fun () -> Utils.drive_index "");
+  assert_raises exn (fun () -> Utils.drive_index "abc123");
+  assert_raises exn (fun () -> Utils.drive_index "123");
+  assert_raises exn (fun () -> Utils.drive_index "Z");
+  assert_raises exn (fun () -> Utils.drive_index "aB")
+
 (* Suites declaration. *)
 let suite    "virt-v2v" >:::
     [
       "OVF.get_ostype" >:: test_get_ostype;
       "Utils.drive_name" >:: test_drive_name;
+      "Utils.drive_index" >:: test_drive_index;
     ]
 
 let () -- 
2.3.1

Richard W.M. Jones

2015-Jul-01 17:54 UTC

head link

[Libguestfs] [PATCH 6/9] v2v: -i libvirt and -i ova: Record the slot number of removable drives.

When we see libvirt source XML for a removable drive like this:

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

as well as recording the bus (s_removable_controller) as we do at the
moment, also record the slot number (eg. hdc == 2 in the example
above).

Do the same for OVA input files.
---
 v2v/input_libvirtxml.ml               | 29 ++++++++++++++++++++++++++++-
 v2v/input_ova.ml                      |  8 +++++---
 v2v/test-v2v-i-ova-formats.expected   |  2 +-
 v2v/test-v2v-i-ova-gz.expected        |  2 +-
 v2v/test-v2v-i-ova-two-disks.expected |  2 +-
 v2v/types.ml                          |  7 +++++--
 v2v/types.mli                         |  1 +
 7 files changed, 42 insertions(+), 9 deletions(-)

diff --git a/v2v/input_libvirtxml.ml b/v2v/input_libvirtxml.ml
index 646346d..653bfc5 100644
--- a/v2v/input_libvirtxml.ml
+++ b/v2v/input_libvirtxml.ml
@@ -33,6 +33,19 @@ and parsed_source  | P_source_file of string
 | P_dont_rewrite
 
+(* Turn string like "hda" into controller slot number.  See also
+ * src/utils.c:guestfs_int_drive_index which this function calls.
+ *)
+let get_drive_slot str offset +  let len = String.length str in
+  if len-offset < 0 then
+    failwith (sprintf "get_drive_slot: offset longer than string length
(offset = %d, string = %s)" offset str);
+  let name = String.sub str offset (len-offset) in
+  try Some (drive_index name)
+  with Invalid_argument _ ->
+       warning (f_"could not parse device name '%s' from the
source libvirt XML") str;
+       None
+
 let parse_libvirt_xml ?conn xml    if verbose () then
     printf "libvirt xml is:\n%s\n" xml;
@@ -291,6 +304,18 @@ let parse_libvirt_xml ?conn xml          |
"virtio" -> Some Source_virtio_blk
         | _ -> None in
 
+      let slot +        let target_dev = xpath_to_string
"target/@dev" "" in
+        match target_dev with
+        | "" -> None
+        | s when string_prefix s "hd" -> get_drive_slot s 2
+        | s when string_prefix s "sd" -> get_drive_slot s 2
+        | s when string_prefix s "vd" -> get_drive_slot s 2
+        | s when string_prefix s "xvd" -> get_drive_slot s 3
+        | s ->
+           warning (f_"<target dev='%s'> was ignored because
the device name could not be recognized") s;
+           None in
+
       let typ          match xpath_to_string "@device" ""
with
         | "cdrom" -> CDROM
@@ -298,7 +323,9 @@ let parse_libvirt_xml ?conn xml          | _ -> assert
false (* libxml2 error? *) in
 
       let disk -        { s_removable_type = typ; s_removable_controller =
controller } in
+        { s_removable_type = typ;
+          s_removable_controller = controller;
+          s_removable_slot = slot } in
       disks := disk :: !disks
     done;
     List.rev !disks in
diff --git a/v2v/input_ova.ml b/v2v/input_ova.ml
index 0ef349d..d640d4a 100644
--- a/v2v/input_ova.ml
+++ b/v2v/input_ova.ml
@@ -321,9 +321,10 @@ object
         let id = xpath_to_int "rasd:ResourceType/text()" 0 in
         assert (id = 14 || id = 15 || id = 16);
 
-        (* XXX We assume the OVF lists these in order.
-        let address = xpath_to_int "rasd:AddressOnParent/text()" 0 in
-        *)
+        let slot +          match xpath_to_int
"rasd:AddressOnParent/text()" (-1) with
+          | -1 -> None
+          | i -> Some i in
 
         (* Find the parent controller. *)
         let parent_id = xpath_to_int "rasd:Parent/text()" 0 in
@@ -340,6 +341,7 @@ object
         let disk = {
           s_removable_type = typ;
           s_removable_controller = controller;
+          s_removable_slot = slot;
         } in
         removables := disk :: !removables;
       done in
diff --git a/v2v/test-v2v-i-ova-formats.expected
b/v2v/test-v2v-i-ova-formats.expected
index 22e6136..c83e5dd 100644
--- a/v2v/test-v2v-i-ova-formats.expected
+++ b/v2v/test-v2v-i-ova-formats.expected
@@ -11,7 +11,7 @@ hypervisor type: vmware
 disks:
 	disk1.vmdk (vmdk) [scsi]
 removable media:
-	CD-ROM [ide]
+	CD-ROM [ide] in slot 0
 NICs:
 	Network "Network adapter 1"
 
diff --git a/v2v/test-v2v-i-ova-gz.expected b/v2v/test-v2v-i-ova-gz.expected
index e6ef699..be6cde3 100644
--- a/v2v/test-v2v-i-ova-gz.expected
+++ b/v2v/test-v2v-i-ova-gz.expected
@@ -11,7 +11,7 @@ hypervisor type: vmware
 disks:
 	.vmdk (vmdk) [scsi]
 removable media:
-	CD-ROM [ide]
+	CD-ROM [ide] in slot 0
 NICs:
 	Network "Network adapter 1"
 
diff --git a/v2v/test-v2v-i-ova-two-disks.expected
b/v2v/test-v2v-i-ova-two-disks.expected
index c45f266..dcbd43e 100644
--- a/v2v/test-v2v-i-ova-two-disks.expected
+++ b/v2v/test-v2v-i-ova-two-disks.expected
@@ -12,7 +12,7 @@ disks:
 	disk1.vmdk (vmdk) [scsi]
 	disk2.vmdk (vmdk) [scsi]
 removable media:
-	CD-ROM [ide]
+	CD-ROM [ide] in slot 0
 NICs:
 	Network "Network adapter 1"
 
diff --git a/v2v/types.ml b/v2v/types.ml
index c583554..34e169c 100644
--- a/v2v/types.ml
+++ b/v2v/types.ml
@@ -55,6 +55,7 @@ and s_controller = Source_IDE | Source_SCSI |
Source_virtio_blk
 and source_removable = {
   s_removable_type : s_removable_type;
   s_removable_controller : s_controller option;
+  s_removable_slot : int option;
 }
 and s_removable_type = CDROM | Floppy
 and source_nic = {
@@ -175,12 +176,14 @@ and string_of_controller = function
   | Source_virtio_blk -> "virtio"
 
 and string_of_source_removable { s_removable_type = typ;
-                                 s_removable_controller = controller } - 
sprintf "\t%s%s"
+                                 s_removable_controller = controller;
+                                 s_removable_slot = i } +  sprintf
"\t%s%s%s"
     (match typ with CDROM -> "CD-ROM" | Floppy ->
"Floppy")
     (match controller with
     | None -> ""
     | Some controller -> " [" ^ string_of_controller controller ^
"]")
+    (match i with None -> "" | Some i -> sprintf " in slot
%d" i)
 
 and string_of_source_nic { s_mac = mac; s_vnet = vnet; s_vnet_type = typ }   
sprintf "\t%s \"%s\"%s"
diff --git a/v2v/types.mli b/v2v/types.mli
index b76ef52..6f9bf0d 100644
--- a/v2v/types.mli
+++ b/v2v/types.mli
@@ -74,6 +74,7 @@ and s_controller = Source_IDE | Source_SCSI |
Source_virtio_blk
 and source_removable = {
   s_removable_type : s_removable_type;  (** Type.  *)
   s_removable_controller : s_controller option; (** Controller, eg. IDE,
SCSI.*)
+  s_removable_slot : int option; (** Slot, eg. hda = 0, hdc = 2 *)
 }
 (** Removable media. *)
 
-- 
2.3.1

Richard W.M. Jones

2015-Jul-01 17:54 UTC

head link

[Libguestfs] [PATCH 7/9] v2v: Introduce the concept of target buses.

The target VM will have several buses to which disks can be attached.
Commonly it will have an IDE bus, and possibly a virtio-blk "bus" (not
really a bus) and/or a SCSI bus.

Virt-v2v does not model this at the moment.  Disks are just added to
the output XML in the order that we write them, and so they can move
around with respect to the target VM.

This commit introduces the idea that we should model the target disk
buses, and we should try to assign fixed and removable disks to slots
on each of those buses.  In this commit, the modelling is not used by
any output mode, but that will be fixed in the next commit.
---
 v2v/types.ml  | 29 +++++++++++++++++++++++
 v2v/types.mli | 14 +++++++++++
 v2v/v2v.ml    | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 119 insertions(+)

diff --git a/v2v/types.ml b/v2v/types.ml
index 34e169c..522814e 100644
--- a/v2v/types.ml
+++ b/v2v/types.ml
@@ -350,6 +350,35 @@ gcaps_acpi = %b
   gcaps.gcaps_arch
   gcaps.gcaps_acpi
 
+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;
+}
+
+and target_bus_slot +  | BusSlotEmpty
+  | BusSlotTarget of target
+  | BusSlotRemovable of source_removable
+
+let string_of_target_bus_slots bus_name slots +  let slots +    Array.mapi (
+      fun slot_nr slot ->
+        sprintf "%s slot %d:\n" bus_name slot_nr ^
+          (match slot with
+           | BusSlotEmpty -> "\t(slot empty)\n"
+           | BusSlotTarget t -> string_of_target t
+           | BusSlotRemovable r -> string_of_source_removable r ^
"\n"
+          )
+    ) slots in
+  String.concat "" (Array.to_list slots)
+
+let string_of_target_buses buses +  string_of_target_bus_slots
"virtio-blk" buses.target_virtio_blk_bus ^
+  string_of_target_bus_slots "ide" buses.target_ide_bus ^
+  string_of_target_bus_slots "scsi" buses.target_scsi_bus
+
 class virtual input = object
   method virtual as_options : string
   method virtual source : unit -> source
diff --git a/v2v/types.mli b/v2v/types.mli
index 6f9bf0d..45014a7 100644
--- a/v2v/types.mli
+++ b/v2v/types.mli
@@ -196,6 +196,20 @@ and guestcaps_video_type = QXL | Cirrus
 
 val string_of_guestcaps : guestcaps -> string
 
+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;
+}
+(** Mapping of fixed and removable disks to buses. *)
+
+and target_bus_slot +| BusSlotEmpty                  (** This bus slot is
empty. *)
+| BusSlotTarget of target       (** Contains a fixed disk. *)
+| BusSlotRemovable of source_removable (** Contains a removable CD/floppy. *)
+
+val string_of_target_buses : target_buses -> string
+
 class virtual input : object
   method virtual as_options : string
   (** Converts the input object back to the equivalent command line options.
diff --git a/v2v/v2v.ml b/v2v/v2v.ml
index 5bca846..61ef4c1 100644
--- a/v2v/v2v.ml
+++ b/v2v/v2v.ml
@@ -311,6 +311,11 @@ let rec main ()    g#shutdown ();
   g#close ();
 
+  message (f_"Assigning disks to buses");
+  let target_buses = target_bus_assignment source targets guestcaps in
+  if verbose () then
+    printf "%s%!" (string_of_target_buses target_buses);
+
   (* Force a GC here, to ensure that we're using the minimum resources
    * as we go into the copy stage.  The particular reason is that
    * Windows conversion may have opened a second libguestfs handle
@@ -833,4 +838,75 @@ and du filename    | line::_ -> (try Some
(Int64.of_string line) with _ -> None)
   | [] -> None
 
+(* Assign fixed and removable disks to target buses, as best we can.
+ * This is not solvable for all guests, but at least avoid overlapping
+ * disks (RHBZ#1238053).  XXX This doesn't do the right thing for PC
+ * legacy floppy devices.
+ *)
+and target_bus_assignment source targets guestcaps +  let virtio_blk_bus = ref
[| |]
+  and ide_bus = ref [| |]
+  and scsi_bus = ref [| |] in
+
+  (* Insert a slot into the bus array, making the array bigger if necessary. *)
+  let insert bus i slot +    let oldbus = !bus in
+    let oldlen = Array.length oldbus in
+    if i >= oldlen then (
+      bus := Array.make (i+1) BusSlotEmpty;
+      Array.blit oldbus 0 !bus 0 oldlen
+    );
+    Array.set !bus i slot
+  in
+
+  (* Insert a slot into the bus, but if the desired slot is not empty, then
+   * increment the slot number until we find an empty one.  Returns
+   * true if we got the desired slot.
+   *)
+  let rec insert_after bus i slot +    let len = Array.length !bus in
+    if i >= len || Array.get !bus i = BusSlotEmpty then (
+      insert bus i slot; true
+    ) else (
+      ignore (insert_after bus (i+1) slot); false
+    )
+  in
+
+  (* Add the fixed disks (targets) to either the virtio-blk or IDE bus,
+   * depending on whether the guest has virtio drivers or not.
+   *)
+  iteri (
+    fun i t ->
+      let t = BusSlotTarget t in
+      match guestcaps.gcaps_block_bus with
+      | Virtio_blk -> insert virtio_blk_bus i t
+      | IDE -> insert ide_bus i t
+  ) targets;
+
+  (* Now try to add the removable disks to the bus at the same slot
+   * they originally occupied, but if the slot is occupied, emit a
+   * a warning and insert the disk in the next empty slot in that bus.
+   *)
+  List.iter (
+    fun r ->
+      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_SCSI -> scsi_bus in
+      match r.s_removable_slot with
+      | None -> ignore (insert_after bus 0 (BusSlotRemovable r))
+      | Some desired_slot_nr ->
+         if not (insert_after bus desired_slot_nr (BusSlotRemovable r)) then
+           warning (f_"removable %s device in slot %d clashes with another
disk, so it has been moved to a higher numbered slot on the same bus.  This may
mean that this removable device has a different name inside the guest (for
example a CD-ROM originally called /dev/hdc might move to /dev/hdd, or from D:
to E: on a Windows guest).")
+                   (match r.s_removable_type with
+                    | CDROM -> s_"CD-ROM"
+                    | Floppy -> s_"floppy disk")
+                   desired_slot_nr
+  ) source.s_removables;
+
+  { target_virtio_blk_bus = !virtio_blk_bus;
+    target_ide_bus = !ide_bus;
+    target_scsi_bus = !scsi_bus }
+
 let () = run_main_and_handle_errors main
-- 
2.3.1

Richard W.M. Jones

2015-Jul-01 17:54 UTC

head link

[Libguestfs] [PATCH 8/9] v2v: Pass target_buses to output object (RHBZ#1238053).

Pass the target_buses assignment to the output#create_metadata method.

Now output modes have a choice: they can either ignore the new
parameter and continue to use the flat list of targets.  This is
suitable for output modes that cannot model multiple buses (eg.
-o glance) or can model it but don't bother (currently -o rhev).

Or they can ignore the flat targets parameter and use the new
target_buses parameter, translating that into the appropriate list of
devices.  This is implemented in this commit for these modes:

 -o libvirt
 -o local
 -o qemu
---
 v2v/output_glance.ml   |  2 +-
 v2v/output_libvirt.ml  | 76 ++++++++++++++++++++++----------------------------
 v2v/output_libvirt.mli |  2 +-
 v2v/output_local.ml    |  4 +--
 v2v/output_null.ml     |  2 +-
 v2v/output_qemu.ml     | 61 +++++++++++++++++++++++++++++-----------
 v2v/output_rhev.ml     |  2 +-
 v2v/output_vdsm.ml     |  2 +-
 v2v/test-v2v-i-ova.xml |  2 +-
 v2v/types.ml           |  2 +-
 v2v/types.mli          |  2 +-
 v2v/v2v.ml             |  3 +-
 12 files changed, 91 insertions(+), 69 deletions(-)

diff --git a/v2v/output_glance.ml b/v2v/output_glance.ml
index ad9ec18..e775229 100644
--- a/v2v/output_glance.ml
+++ b/v2v/output_glance.ml
@@ -62,7 +62,7 @@ object
         { t with target_file = target_file }
     ) targets
 
-  method create_metadata source targets guestcaps inspect target_firmware + 
method create_metadata source targets _ guestcaps inspect target_firmware     
(* See #supported_firmware above. *)
     assert (target_firmware = TargetBIOS);
 
diff --git a/v2v/output_libvirt.ml b/v2v/output_libvirt.ml
index 7f02e45..cc18580 100644
--- a/v2v/output_libvirt.ml
+++ b/v2v/output_libvirt.ml
@@ -69,7 +69,7 @@ let target_features_of_capabilities_doc doc arch     
!features
   )
 
-let create_libvirt_xml ?pool source targets guestcaps
+let create_libvirt_xml ?pool source target_buses guestcaps
                        target_features target_firmware    let memory_k =
source.s_memory /^ 1024L in
 
@@ -124,16 +124,12 @@ let create_libvirt_xml ?pool source targets guestcaps
 
     (e "type" ["arch", guestcaps.gcaps_arch] [PCData
"hvm"]) :: loader in
 
-  (* Disks. *)
+  (* Fixed and removable disks. *)
   let disks -    let block_prefix -      match guestcaps.gcaps_block_bus with
-      | Virtio_blk -> "vd" | IDE -> "hd" in
-    let block_bus -      match guestcaps.gcaps_block_bus with
-      | Virtio_blk -> "virtio" | IDE -> "ide" in
-    mapi (
-      fun i t ->
+    let make_disk bus_name drive_prefix i = function
+    | BusSlotEmpty -> Comment (sprintf "%s slot %d is empty"
bus_name i)
+
+    | BusSlotTarget t ->
         e "disk" [
           "type", if pool = None then "file" else
"volume";
           "device", "disk"
@@ -155,45 +151,41 @@ let create_libvirt_xml ?pool source targets guestcaps
             ] []
           );
           e "target" [
-            "dev", block_prefix ^ (drive_name i);
-            "bus", block_bus;
+            "dev", drive_prefix ^ drive_name i;
+            "bus", bus_name;
           ] [];
         ]
-    ) targets in
 
-  let removables -    (* CDs will be added as IDE devices if we're using
virtio, else
-     * they will be added as the same as the disk bus.  The original
-     * s_removable_controller is ignored (same as old virt-v2v).
-     *)
-    let cdrom_bus, cdrom_block_prefix, cdrom_index -      match
guestcaps.gcaps_block_bus with
-      | Virtio_blk | IDE -> "ide", "hd", ref 0
-      (* | bus -> bus, "sd", ref (List.length targets) *) in
-
-    (* Floppy disks always occupy their own virtual bus. *)
-    let fd_bus = "fdc" and fd_index = ref 0 in
-
-    List.map (
-      function
-      | { s_removable_type = CDROM } ->
-        let i = !cdrom_index in
-        incr cdrom_index;
-        let name = cdrom_block_prefix ^ drive_name i in
+    | BusSlotRemovable { s_removable_type = CDROM } ->
         e "disk" [ "device", "cdrom";
"type", "file" ] [
           e "driver" [ "name", "qemu";
"type", "raw" ] [];
-          e "target" [ "dev", name; "bus",
cdrom_bus ] []
+          e "target" [
+            "dev", drive_prefix ^ drive_name i;
+            "bus", bus_name
+          ] []
         ]
 
-      | { s_removable_type = Floppy } ->
-        let i = !fd_index in
-        incr fd_index;
-        let name = "fd" ^ drive_name i in
+    | BusSlotRemovable { s_removable_type = Floppy } ->
         e "disk" [ "device", "floppy";
"type", "file" ] [
           e "driver" [ "name", "qemu";
"type", "raw" ] [];
-          e "target" [ "dev", name; "bus", fd_bus
] []
+          e "target" [
+            "dev", drive_prefix ^ drive_name i;
+            "bus", bus_name
+          ] []
         ]
-    ) source.s_removables in
+    in
+
+    List.flatten [
+      Array.to_list
+        (Array.mapi (make_disk "virtio" "vd")
+                    target_buses.target_virtio_blk_bus);
+      Array.to_list
+        (Array.mapi (make_disk "ide" "hd")
+                    target_buses.target_ide_bus);
+      Array.to_list
+        (Array.mapi (make_disk "scsi" "sd")
+                    target_buses.target_scsi_bus)
+    ] in
 
   let nics      let net_model @@ -278,7 +270,7 @@ let create_libvirt_xml ?pool
source targets guestcaps
        else
          [] in
 
-  let devices = disks @ removables @ nics @ [video] @ [graphics] @ sound @
+  let devices = disks @ nics @ [video] @ [graphics] @ sound @
   (* Standard devices added to every guest. *) [
     e "input" ["type", "tablet"; "bus",
"usb"] [];
     e "input" ["type", "mouse"; "bus",
"ps2"] [];
@@ -376,7 +368,7 @@ class output_libvirt oc output_pool = object
         { t with target_file = target_file }
     ) targets
 
-  method create_metadata source targets guestcaps _ target_firmware +  method
create_metadata source _ target_buses guestcaps _ target_firmware      (* We
copied directly into the final pool directory.  However we
      * have to tell libvirt.
      *)
@@ -400,7 +392,7 @@ class output_libvirt oc output_pool = object
 
     (* Create the metadata. *)
     let doc -      create_libvirt_xml ~pool:output_pool source targets
+      create_libvirt_xml ~pool:output_pool source target_buses
         guestcaps target_features target_firmware in
 
     let tmpfile, chan = Filename.open_temp_file "v2vlibvirt"
".xml" in
diff --git a/v2v/output_libvirt.mli b/v2v/output_libvirt.mli
index 9f2c20b..471a2e8 100644
--- a/v2v/output_libvirt.mli
+++ b/v2v/output_libvirt.mli
@@ -23,5 +23,5 @@ val output_libvirt : string option -> string ->
Types.output
     {!Types.output} object specialized for writing output to
     libvirt. *)
 
-val create_libvirt_xml : ?pool:string -> Types.source -> Types.target
list -> Types.guestcaps -> string list -> Types.target_firmware ->
DOM.doc
+val create_libvirt_xml : ?pool:string -> Types.source ->
Types.target_buses -> Types.guestcaps -> string list ->
Types.target_firmware -> DOM.doc
 (** This is called from {!Output_local} to generate the libvirt XML. *)
diff --git a/v2v/output_local.ml b/v2v/output_local.ml
index 0e82a3a..c8db359 100644
--- a/v2v/output_local.ml
+++ b/v2v/output_local.ml
@@ -38,7 +38,7 @@ class output_local dir = object
         { t with target_file = target_file }
     ) targets
 
-  method create_metadata source targets guestcaps _ target_firmware +  method
create_metadata source _ target_buses guestcaps _ target_firmware      (* We
don't know what target features the hypervisor supports, but
      * assume a common set that libvirt supports.
      *)
@@ -49,7 +49,7 @@ class output_local dir = object
       | _ -> [] in
 
     let doc -      Output_libvirt.create_libvirt_xml source targets
+      Output_libvirt.create_libvirt_xml source target_buses
         guestcaps target_features target_firmware in
 
     let name = source.s_name in
diff --git a/v2v/output_null.ml b/v2v/output_null.ml
index 5cc89a2..2cada46 100644
--- a/v2v/output_null.ml
+++ b/v2v/output_null.ml
@@ -48,7 +48,7 @@ object
         { t with target_file = target_file }
     ) targets
 
-  method create_metadata _ _ _ _ _ = ()
+  method create_metadata _ _ _ _ _ _ = ()
 end
 
 let output_null () = new output_null
diff --git a/v2v/output_qemu.ml b/v2v/output_qemu.ml
index 81d819e..3b782ba 100644
--- a/v2v/output_qemu.ml
+++ b/v2v/output_qemu.ml
@@ -40,7 +40,8 @@ object
         { t with target_file = target_file }
     ) targets
 
-  method create_metadata source targets guestcaps inspect target_firmware + 
method create_metadata source _ target_buses guestcaps inspect
+                         target_firmware      let name = source.s_name in
     let file = dir // name ^ ".sh" in
 
@@ -82,22 +83,50 @@ object
     if source.s_vcpu > 1 then
       fpf "%s-smp %d" nl source.s_vcpu;
 
-    let block_bus -      match guestcaps.gcaps_block_bus with
-      | Virtio_blk -> "virtio"
-      | IDE -> "ide" in
-    List.iter (
-      fun t ->
-        let qemu_quoted_filename = replace_str t.target_file ","
",," in
-        let drive_param -          sprintf "file=%s,format=%s,if=%s"
-            qemu_quoted_filename t.target_format block_bus in
-        fpf "%s-drive %s" nl (quote drive_param)
-    ) targets;
+    let make_disk if_name i = function
+    | BusSlotEmpty -> ()
 
-    (* XXX Missing:
-     * - removable devices
-     *)
+    | BusSlotTarget t ->
+       let qemu_quoted_filename = replace_str t.target_file ","
",," in
+       let drive_param +          sprintf
"file=%s,format=%s,if=%s,index=%d,media=disk"
+                  qemu_quoted_filename t.target_format if_name i in
+       fpf "%s-drive %s" nl (quote drive_param)
+
+    | BusSlotRemovable { s_removable_type = CDROM } ->
+       let drive_param +          sprintf
"format=raw,if=%s,index=%d,media=cdrom" if_name i in
+       fpf "%s-drive %s" nl (quote drive_param)
+
+    | BusSlotRemovable { s_removable_type = Floppy } ->
+       let drive_param +          sprintf
"format=raw,if=%s,index=%d,media=floppy" if_name i in
+       fpf "%s-drive %s" nl (quote drive_param)
+    in
+    Array.iteri (make_disk "virtio")
target_buses.target_virtio_blk_bus;
+    Array.iteri (make_disk "ide") target_buses.target_ide_bus;
+
+    let make_scsi i = function
+    | BusSlotEmpty -> ()
+
+    | BusSlotTarget t ->
+       let qemu_quoted_filename = replace_str t.target_file ","
",," in
+       let drive_param +          sprintf
"file=%s,format=%s,if=scsi,bus=0,unit=%d,media=disk"
+                  qemu_quoted_filename t.target_format i in
+       fpf "%s-drive %s" nl (quote drive_param)
+
+    | BusSlotRemovable { s_removable_type = CDROM } ->
+       let drive_param +          sprintf
"format=raw,if=scsi,bus=0,unit=%d,media=cdrom" i in
+       fpf "%s-drive %s" nl (quote drive_param)
+
+    | BusSlotRemovable { s_removable_type = Floppy } ->
+       let drive_param +          sprintf
"format=raw,if=scsi,bus=0,unit=%d,media=floppy" i in
+       fpf "%s-drive %s" nl (quote drive_param)
+    in
+    Array.iteri make_scsi target_buses.target_scsi_bus;
 
     let net_bus        match guestcaps.gcaps_net_bus with
diff --git a/v2v/output_rhev.ml b/v2v/output_rhev.ml
index 365c35e..d3f367e 100644
--- a/v2v/output_rhev.ml
+++ b/v2v/output_rhev.ml
@@ -278,7 +278,7 @@ object
     )
 
   (* This is called after conversion to write the OVF metadata. *)
-  method create_metadata source targets guestcaps inspect target_firmware + 
method create_metadata source targets _ guestcaps inspect target_firmware     
(* See #supported_firmware above. *)
     assert (target_firmware = TargetBIOS);
 
diff --git a/v2v/output_vdsm.ml b/v2v/output_vdsm.ml
index 44f0041..670d8ba 100644
--- a/v2v/output_vdsm.ml
+++ b/v2v/output_vdsm.ml
@@ -165,7 +165,7 @@ object
       ?clustersize path format size
 
   (* This is called after conversion to write the OVF metadata. *)
-  method create_metadata source targets guestcaps inspect target_firmware + 
method create_metadata source targets _ guestcaps inspect target_firmware     
(* See #supported_firmware above. *)
     assert (target_firmware = TargetBIOS);
 
diff --git a/v2v/test-v2v-i-ova.xml b/v2v/test-v2v-i-ova.xml
index 994727b..2eeff83 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='fda' bus='fdc'/>
+      <target dev='hdb' bus='ide'/>
     </disk>
     <interface type='network'>
       <source network='Ethernet 1'/>
diff --git a/v2v/types.ml b/v2v/types.ml
index 522814e..9a0b24b 100644
--- a/v2v/types.ml
+++ b/v2v/types.ml
@@ -391,7 +391,7 @@ class virtual output = object
   method virtual supported_firmware : target_firmware list
   method check_target_free_space (_ : source) (_ : target list) = ()
   method disk_create = (new Guestfs.guestfs ())#disk_create
-  method virtual create_metadata : source -> target list -> guestcaps
-> inspect -> target_firmware -> unit
+  method virtual create_metadata : source -> target list -> target_buses
-> guestcaps -> inspect -> target_firmware -> unit
   method keep_serial_console = true
 end
 
diff --git a/v2v/types.mli b/v2v/types.mli
index 45014a7..70c6fc7 100644
--- a/v2v/types.mli
+++ b/v2v/types.mli
@@ -234,7 +234,7 @@ class virtual output : object
   method check_target_free_space : source -> target list -> unit
   (** Called before conversion.  Can be used to check there is enough space
       on the target, using the [target.target_estimated_size] field. *)
-  method virtual create_metadata : source -> target list -> guestcaps
-> inspect -> target_firmware -> unit
+  method virtual create_metadata : source -> target list -> target_buses
-> guestcaps -> inspect -> target_firmware -> unit
   (** Called after conversion to finish off and create metadata. *)
   method disk_create : ?backingfile:string -> ?backingformat:string ->
?preallocation:string -> ?compat:string -> ?clustersize:int -> string
-> string -> int64 -> unit
   (** Called in order to create disks on the target.  The method has the
diff --git a/v2v/v2v.ml b/v2v/v2v.ml
index 61ef4c1..bee0377 100644
--- a/v2v/v2v.ml
+++ b/v2v/v2v.ml
@@ -444,7 +444,8 @@ let rec main ()  
   (* Create output metadata. *)
   message (f_"Creating output metadata");
-  output#create_metadata source targets guestcaps inspect target_firmware;
+  output#create_metadata source targets target_buses guestcaps inspect
+                         target_firmware;
 
   (* Save overlays if --debug-overlays option was used. *)
   if debug_overlays then (
-- 
2.3.1

Richard W.M. Jones

2015-Jul-01 17:54 UTC

head link

[Libguestfs] [PATCH 9/9] v2v: Add a regression test for RHBZ#1238053.

---
 v2v/Makefile.am             |  3 ++
 v2v/test-v2v-cdrom.expected |  8 +++++
 v2v/test-v2v-cdrom.sh       | 80 +++++++++++++++++++++++++++++++++++++++++++++
 v2v/test-v2v-cdrom.xml      | 43 ++++++++++++++++++++++++
 4 files changed, 134 insertions(+)
 create mode 100644 v2v/test-v2v-cdrom.expected
 create mode 100755 v2v/test-v2v-cdrom.sh
 create mode 100644 v2v/test-v2v-cdrom.xml

diff --git a/v2v/Makefile.am b/v2v/Makefile.am
index 0dd9d9b..8932057 100644
--- a/v2v/Makefile.am
+++ b/v2v/Makefile.am
@@ -24,6 +24,8 @@ EXTRA_DIST = \
 	HACKING \
 	rhel-7-virt-v2v-conversion-server.config \
 	rhel-7-virt-v2v-conversion-server.ks \
+	test-v2v-cdrom.expected \
+	test-v2v-cdrom.xml \
 	test-v2v-i-ova.ovf \
 	test-v2v-i-ova.xml \
 	test-v2v-i-ova-formats.expected \
@@ -229,6 +231,7 @@ endif
 
 if ENABLE_APPLIANCE
 TESTS += \
+	test-v2v-cdrom.sh \
 	test-v2v-i-ova.sh \
 	test-v2v-i-disk.sh \
 	test-v2v-machine-readable.sh \
diff --git a/v2v/test-v2v-cdrom.expected b/v2v/test-v2v-cdrom.expected
new file mode 100644
index 0000000..e18ea6f
--- /dev/null
+++ b/v2v/test-v2v-cdrom.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='cdrom' type='file'>
+      <driver name='qemu' type='raw'/>
+      <target dev='hdc' bus='ide'/>
+    </disk>
diff --git a/v2v/test-v2v-cdrom.sh b/v2v/test-v2v-cdrom.sh
new file mode 100755
index 0000000..93975c2
--- /dev/null
+++ b/v2v/test-v2v-cdrom.sh
@@ -0,0 +1,80 @@
+#!/bin/bash -
+# libguestfs virt-v2v test script
+# Copyright (C) 2015 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 cdrom dev assignment.
+# https://bugzilla.redhat.com/show_bug.cgi?id=1238053
+
+unset CDPATH
+export LANG=C
+set -e
+
+if [ -n "$SKIP_TEST_V2V_CDROM_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-cdrom.xml"
+
+f=../tests/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=../tests/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
+
+virt_tools_data_dir=${VIRT_TOOLS_DATA_DIR:-/usr/share/virt-tools}
+if ! test -r $virt_tools_data_dir/rhsrvany.exe; then
+    echo "$0: test skipped because rhsrvany.exe is not installed"
+    exit 77
+fi
+
+d=test-v2v-cdrom.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-cdrom.expected $d/disks; then
+    echo "$0: unexpected disk assignments"
+    cat $d/disks
+    exit 1
+fi
+
+rm -r $d
diff --git a/v2v/test-v2v-cdrom.xml b/v2v/test-v2v-cdrom.xml
new file mode 100644
index 0000000..b48d88f
--- /dev/null
+++ b/v2v/test-v2v-cdrom.xml
@@ -0,0 +1,43 @@
+<!--
+libguestfs virt-v2v tool
+Copyright (C) 2009-2015 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='../tests/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='cdrom'>
+        <driver name='qemu' type='raw'/>
+        <source file='../tests/guests/blank-disk.img'/>
+        <!-- virt-v2v should preserve the device name and bus -->
+        <target dev='hdc' bus='ide'/>
+      </disk>
+    </devices>
+  </domain>
+</node>
-- 
2.3.1

Pino Toscano

2015-Jul-02 08:56 UTC

head link

Re: [Libguestfs] [PATCH 7/9] v2v: Introduce the concept of target buses.

On Wednesday 01 July 2015 18:54:49 Richard W.M. Jones
wrote:> The target VM will have several buses to which disks can be attached.
> Commonly it will have an IDE bus, and possibly a virtio-blk "bus"
(not
> really a bus) and/or a SCSI bus.
> 
> Virt-v2v does not model this at the moment.  Disks are just added to
> the output XML in the order that we write them, and so they can move
> around with respect to the target VM.
> 
> This commit introduces the idea that we should model the target disk
> buses, and we should try to assign fixed and removable disks to slots
> on each of those buses.  In this commit, the modelling is not used by
> any output mode, but that will be fixed in the next commit.
> ---
> [...]
> +  (* Now try to add the removable disks to the bus at the same slot
> +   * they originally occupied, but if the slot is occupied, emit a
> +   * a warning and insert the disk in the next empty slot in that bus.
> +   *)
> +  List.iter (
> +    fun r ->
> +      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_SCSI -> scsi_bus in
> +      match r.s_removable_slot with
> +      | None -> ignore (insert_after bus 0 (BusSlotRemovable r))
> +      | Some desired_slot_nr ->
> +         if not (insert_after bus desired_slot_nr (BusSlotRemovable r))
then
> +           warning (f_"removable %s device in slot %d clashes with
another disk, so it has been moved to a higher numbered slot on the same bus. 
This may mean that this removable device has a different name inside the guest
(for example a CD-ROM originally called /dev/hdc might move to /dev/hdd, or from
D: to E: on a Windows guest).")
> +                   (match r.s_removable_type with
> +                    | CDROM -> s_"CD-ROM"
> +                    | Floppy -> s_"floppy disk")
> +                   desired_slot_nr
Imagine that first we get a source with no specified slot, so we insert
it at 0, and then we get a source which specifies the slot zero which
is then taken.
Maybe this iteration should be split into two separate, first adding all
the sources which specify a slot, and then all the ones which have no
specific slot. This way there is more probability to assign the slots
matching how they were originally, I think.

-- 
Pino Toscano

Pino Toscano

2015-Jul-02 09:06 UTC

head link

Re: [Libguestfs] [PATCH 1/9] v2v: Stable bus and slot numbers for removable drives (RHBZ#1238053).

On Wednesday 01 July 2015 18:54:42 Richard W.M. Jones
wrote:> This patch series adds stable bus and slot numbers for removable
> drives (CDs and floppies) when the guest is converted using virt-v2v
> or virt-p2v.
> 
> Previously we were a bit random about this.  After this patch series,
> the bus and slot numbers and preserved if at all possible.
> 
> BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1238053
Patches 1-6 are fine.
Patch 7 has a note.
Patches 8-9 seems fine too, at a first glance.

Thanks,
-- 
Pino Toscano

Richard W.M. Jones

2015-Jul-02 13:17 UTC

head link

Re: [Libguestfs] [PATCH 1/9] v2v: Stable bus and slot numbers for removable drives (RHBZ#1238053).

On Thu, Jul 02, 2015 at 11:06:46AM +0200, Pino Toscano
wrote:> On Wednesday 01 July 2015 18:54:42 Richard W.M. Jones wrote:
> > This patch series adds stable bus and slot numbers for removable
> > drives (CDs and floppies) when the guest is converted using virt-v2v
> > or virt-p2v.
> > 
> > Previously we were a bit random about this.  After this patch series,
> > the bus and slot numbers and preserved if at all possible.
> > 
> > BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1238053
> 
> Patches 1-6 are fine.
> Patch 7 has a note.
> Patches 8-9 seems fine too, at a first glance.
I have pushed this with a to-do comment on patch 7 (on the basis that
it doesn't make things worse than they are now - at the moment,
devices are effectively randomized).

Thanks,

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

Possibly Parallel Threads

Search for more possibly parallel threads

Libguestfs - Jul 2015 - Re: [PATCH 1/9] v2v: Stable bus and slot numbers for removable drives (RHBZ#1238053).

[Libguestfs] [PATCH 1/9] v2v: Stable bus and slot numbers for removable drives (RHBZ#1238053).

[Libguestfs] [PATCH 1/9] utils: Add a test of the guestfs_int_drive_name function.

[Libguestfs] [PATCH 2/9] utils: Add guestfs_int_drive_index and unit test.

[Libguestfs] [PATCH 3/9] v2v: Refactor ~printer parameter in unit tests.

[Libguestfs] [PATCH 4/9] v2v: Add unit test of Utils.drive_name function.

[Libguestfs] [PATCH 5/9] v2v: Add a binding for guestfs_int_drive_index and a unit test.

[Libguestfs] [PATCH 6/9] v2v: -i libvirt and -i ova: Record the slot number of removable drives.

[Libguestfs] [PATCH 7/9] v2v: Introduce the concept of target buses.

[Libguestfs] [PATCH 8/9] v2v: Pass target_buses to output object (RHBZ#1238053).

[Libguestfs] [PATCH 9/9] v2v: Add a regression test for RHBZ#1238053.

Re: [Libguestfs] [PATCH 7/9] v2v: Introduce the concept of target buses.

Re: [Libguestfs] [PATCH 1/9] v2v: Stable bus and slot numbers for removable drives (RHBZ#1238053).

Re: [Libguestfs] [PATCH 1/9] v2v: Stable bus and slot numbers for removable drives (RHBZ#1238053).

Possibly Parallel Threads