From ab0f4d6925bfac4cb65c6d6e6e050d5366a8163b Mon Sep 17 00:00:00 2001 From: "Richard W.M. Jones" 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 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 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 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; (** *) +} +and parsed_source = +| P_source_dev of string (** *) +| P_source_file of string (** *) +| 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