Blob Blame History Raw
From ab0f4d6925bfac4cb65c6d6e6e050d5366a8163b Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Wed, 22 Oct 2014 12:00:36 +0100
Subject: [PATCH] v2v: -i libvirtxml: Fix handling of nbd sources
 (RHBZ#1153589).

Previously I modified the parse_libvirt_xml function to get rid of the
awkward 'map_source*' functions, and have the callers map over and
modify the source disks afterwards.  However this broke the case where
an "nbd:..." URL was returned by parse_libvirt_xml, since the callers
might try to map this URL (eg. turning it into an absolute path).
This broke virt-p2v specifically.

This commit changes parse_libvirt_xml to return a list of tuples
containing disk information, giving the callers more information with
which to do the mapping.

This fixes commit 3596165282ccf2c5896894ec4e9a71c6da788463.

(cherry picked from commit ad78d1492b02eaf5d810e2f9d012a5fed4f4124b)
---
 v2v/input_libvirt_other.ml         |  5 ++++-
 v2v/input_libvirt_vcenter_https.ml | 44 ++++++++++++++++++++++++--------------
 v2v/input_libvirt_xen_ssh.ml       | 10 ++++++---
 v2v/input_libvirtxml.ml            | 38 +++++++++++++++++++++-----------
 v2v/input_libvirtxml.mli           | 17 +++++++++++++--
 5 files changed, 80 insertions(+), 34 deletions(-)

diff --git a/v2v/input_libvirt_other.ml b/v2v/input_libvirt_other.ml
index a771aa1..9f3eedb 100644
--- a/v2v/input_libvirt_other.ml
+++ b/v2v/input_libvirt_other.ml
@@ -70,7 +70,10 @@ object
      *)
     let xml = Domainxml.dumpxml ?conn:libvirt_uri guest in
 
-    Input_libvirtxml.parse_libvirt_xml ~verbose xml
+    let source, disks = Input_libvirtxml.parse_libvirt_xml ~verbose xml in
+    let disks =
+      List.map (fun { Input_libvirtxml.p_source_disk = disk } -> disk) disks in
+    { source with s_disks = disks }
 end
 
 let input_libvirt_other = new input_libvirt_other
diff --git a/v2v/input_libvirt_vcenter_https.ml b/v2v/input_libvirt_vcenter_https.ml
index 71c2edd..56097e0 100644
--- a/v2v/input_libvirt_vcenter_https.ml
+++ b/v2v/input_libvirt_vcenter_https.ml
@@ -24,6 +24,7 @@ open Common_utils
 open Types
 open Xml
 open Utils
+open Input_libvirtxml
 open Input_libvirt_other
 
 open Printf
@@ -235,20 +236,28 @@ object
      * that the domain is not running.  (RHBZ#1138586)
      *)
     let xml = Domainxml.dumpxml ?conn:libvirt_uri guest in
-    let { s_disks = disks } as source =
-      Input_libvirtxml.parse_libvirt_xml ~verbose xml in
+    let source, disks = parse_libvirt_xml ~verbose xml in
 
     (* Save the original source paths, so that we can remap them again
      * in [#adjust_overlay_parameters].
      *)
     List.iter (
-      fun { s_disk_id = id; s_qemu_uri = path } ->
-        Hashtbl.add saved_source_paths id path
+      function
+      | { p_source = P_source_dev _ } ->
+        (* Should never happen ... *)
+        error (f_"source disk has <source dev=...> attribute in XML")
+      | { p_source_disk = { s_disk_id = id }; p_source = P_dont_rewrite } ->
+        Hashtbl.add saved_source_paths id None
+      | { p_source_disk = { s_disk_id = id }; p_source = P_source_file path } ->
+        Hashtbl.add saved_source_paths id (Some path)
     ) disks;
 
+    let readahead = readahead_for_conversion in
     let disks = List.map (
-      fun ({ s_qemu_uri = path } as disk) ->
-        let readahead = readahead_for_conversion in
+      function
+      | { p_source = P_source_dev _ } -> assert false
+      | { p_source_disk = disk; p_source = P_dont_rewrite } -> disk
+      | { p_source_disk = disk; p_source = P_source_file path } ->
         let qemu_uri = map_source_to_uri ?readahead
 	  verbose parsed_uri scheme server path in
 
@@ -265,18 +274,21 @@ object
     let orig_path =
       try Hashtbl.find saved_source_paths overlay.ov_source.s_disk_id
       with Not_found -> failwith "internal error in adjust_overlay_parameters" in
-    let backing_qemu_uri =
+    match orig_path with
+    | None -> ()
+    | Some orig_path ->
       let readahead = readahead_for_copying in
-      map_source_to_uri ?readahead
-        verbose parsed_uri scheme server orig_path in
+      let backing_qemu_uri =
+        map_source_to_uri ?readahead
+          verbose parsed_uri scheme server orig_path in
 
-    (* Rebase the qcow2 overlay to adjust the readahead parameter. *)
-    let cmd =
-      sprintf "qemu-img rebase -u -b %s %s"
-        (quote backing_qemu_uri) (quote overlay.ov_overlay_file) in
-    if verbose then printf "%s\n%!" cmd;
-    if Sys.command cmd <> 0 then
-      warning ~prog (f_"qemu-img rebase failed (ignored)")
+      (* Rebase the qcow2 overlay to adjust the readahead parameter. *)
+      let cmd =
+        sprintf "qemu-img rebase -u -b %s %s"
+          (quote backing_qemu_uri) (quote overlay.ov_overlay_file) in
+      if verbose then printf "%s\n%!" cmd;
+      if Sys.command cmd <> 0 then
+        warning ~prog (f_"qemu-img rebase failed (ignored)")
 end
 
 let input_libvirt_vcenter_https = new input_libvirt_vcenter_https
diff --git a/v2v/input_libvirt_xen_ssh.ml b/v2v/input_libvirt_xen_ssh.ml
index 8b836a5..e1600a0 100644
--- a/v2v/input_libvirt_xen_ssh.ml
+++ b/v2v/input_libvirt_xen_ssh.ml
@@ -24,6 +24,7 @@ open Common_utils
 open Types
 open Xml
 open Utils
+open Input_libvirtxml
 open Input_libvirt_other
 
 open Printf
@@ -45,8 +46,7 @@ object
      * that the domain is not running.  (RHBZ#1138586)
      *)
     let xml = Domainxml.dumpxml ?conn:libvirt_uri guest in
-    let { s_disks = disks } as source =
-      Input_libvirtxml.parse_libvirt_xml ~verbose xml in
+    let source, disks = parse_libvirt_xml ~verbose xml in
 
     (* Map the <source/> filename (which is relative to the remote
      * Xen server) to an ssh URI.  This is a JSON URI looking something
@@ -61,7 +61,11 @@ object
      *   "file.host_key_check": "no"
      *)
     let disks = List.map (
-      fun ({ s_qemu_uri = path } as disk) ->
+      function
+      | { p_source_disk = disk; p_source = P_dont_rewrite } ->
+        disk
+      | { p_source_disk = disk; p_source = P_source_dev path }
+      | { p_source_disk = disk; p_source = P_source_file path } ->
         (* Construct the JSON parameters. *)
         let json_params = [
           "file.driver", JSON.String "ssh";
diff --git a/v2v/input_libvirtxml.ml b/v2v/input_libvirtxml.ml
index 0cfd75c..c71260f 100644
--- a/v2v/input_libvirtxml.ml
+++ b/v2v/input_libvirtxml.ml
@@ -24,6 +24,15 @@ open Common_utils
 open Types
 open Utils
 
+type parsed_disk = {
+  p_source_disk : source_disk;
+  p_source : parsed_source;
+}
+and parsed_source =
+| P_source_dev of string
+| P_source_file of string
+| P_dont_rewrite
+
 let parse_libvirt_xml ~verbose xml =
   if verbose then
     printf "libvirt xml is:\n%s\n" xml;
@@ -105,12 +114,13 @@ let parse_libvirt_xml ~verbose xml =
     let get_disks, add_disk =
       let disks = ref [] and i = ref 0 in
       let get_disks () = List.rev !disks in
-      let add_disk qemu_uri format target_dev =
+      let add_disk qemu_uri format target_dev p_source =
         incr i;
         disks :=
-          { s_disk_id = !i;
-            s_qemu_uri = qemu_uri; s_format = format;
-            s_target_dev = target_dev } :: !disks
+          { p_source_disk = { s_disk_id = !i;
+                              s_qemu_uri = qemu_uri; s_format = format;
+                              s_target_dev = target_dev };
+            p_source = p_source } :: !disks
       in
       get_disks, add_disk
     in
@@ -141,11 +151,11 @@ let parse_libvirt_xml ~verbose xml =
       | "block" ->
         let path = xpath_to_string "source/@dev" "" in
         if path <> "" then
-          add_disk path format target_dev
+          add_disk path format target_dev (P_source_dev path)
       | "file" ->
         let path = xpath_to_string "source/@file" "" in
         if path <> "" then
-          add_disk path format target_dev
+          add_disk path format target_dev (P_source_file path)
       | "network" ->
         (* We only handle <source protocol="nbd"> here, and that is
          * intended only for virt-p2v.  Any other network disk is
@@ -160,7 +170,7 @@ let parse_libvirt_xml ~verbose xml =
              * XXX Quoting, although it's not needed for virt-p2v.
              *)
             let path = sprintf "nbd:%s:%d" host port in
-            add_disk path format target_dev
+            add_disk path format target_dev (P_dont_rewrite)
           )
         | "" -> ()
         | protocol ->
@@ -236,17 +246,18 @@ let parse_libvirt_xml ~verbose xml =
     done;
     List.rev !nics in
 
-  {
+  ({
     s_dom_type = dom_type;
     s_name = name; s_orig_name = name;
     s_memory = memory;
     s_vcpu = vcpu;
     s_features = features;
     s_display = display;
-    s_disks = disks;
+    s_disks = [];
     s_removables = removables;
     s_nics = nics;
-  }
+   },
+   disks)
 
 class input_libvirtxml verbose file =
 object
@@ -257,7 +268,7 @@ object
   method source () =
     let xml = read_whole_file file in
 
-    let { s_disks = disks } as source = parse_libvirt_xml ~verbose xml in
+    let source, disks = parse_libvirt_xml ~verbose xml in
 
     (* When reading libvirt XML from a file (-i libvirtxml) we allow
      * paths to disk images in the libvirt XML to be relative (to the XML
@@ -267,7 +278,10 @@ object
      *)
     let dir = Filename.dirname (absolute_path file) in
     let disks = List.map (
-      fun ({ s_qemu_uri = path } as disk) ->
+      function
+      | { p_source_disk = disk; p_source = P_dont_rewrite } -> disk
+      | { p_source_disk = disk; p_source = P_source_dev _ } -> disk
+      | { p_source_disk = disk; p_source = P_source_file path } ->
         let path =
           if not (Filename.is_relative path) then path else dir // path in
         { disk with s_qemu_uri = path }
diff --git a/v2v/input_libvirtxml.mli b/v2v/input_libvirtxml.mli
index 5c10df0..e450899 100644
--- a/v2v/input_libvirtxml.mli
+++ b/v2v/input_libvirtxml.mli
@@ -18,8 +18,21 @@
 
 (** [-i libvirtxml] source. *)
 
-val parse_libvirt_xml : verbose:bool -> string -> Types.source
-(** Take libvirt XML and parse it into a {!Types.source} structure.
+type parsed_disk = {
+  p_source_disk : Types.source_disk;    (** Source disk. *)
+  p_source : parsed_source;         (** <source dev|file attribute> *)
+}
+and parsed_source =
+| P_source_dev of string             (** <source dev> *)
+| P_source_file of string            (** <source file> *)
+| P_dont_rewrite                     (** s_qemu_uri is already set. *)
+
+val parse_libvirt_xml : verbose:bool -> string -> Types.source * parsed_disk list
+(** Take libvirt XML and parse it into a {!Types.source} structure and a
+    list of source disks.
+
+    {b Note} the [source.s_disks] field is an empty list.  The caller
+    must map over the parsed disks and update the [source.s_disks] field.
 
     This function is also used by {!Input_libvirt}, hence it is
     exported. *)
-- 
1.8.3.1