[virt-v2v PATCH 0/3] lib/create_ovf: populate "actual size" attributes again
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2027598 With the modularization of virt-v2v, determining the host disk allocation of copied (= output) VM disk images became more difficult, and so we dropped the following attributes from the generated OVF files: - ovf:Envelope/References/File/@ovf:size - ovf:Envelope/Section[@xsi:type='ovf:DiskSection_Type']/Disk/@ovf:actual_size Right now we have an "XXX" reminder in the code. Unfortunately, ovirt-engine really insists on the presence of the "ovf:actual_size" attribute -- there used to be a nullity check regarding the attribute, but it was regressed in ovirt-engine commit commit 1082d9dec289 ("core: undo recent generalization of ovf processing", 2017-08-09). Therefore, as of commit a2e74bdb3fe1 ("core: test network stats with big integer values", 2021-12-07), ovirt-engine rejects our OVF files, and we must generate "ovf:actual_size" even if we don't have the necessary info for it -- that is, with empty contents. This patch series implements -- based on Rich's NBD demo code he had sent me off-list -- the necessary "du"-like logic on the output disks using NBD APIs. Thanks, Laszlo Laszlo Ersek (3): lib/nbdkit: add the "Nbdkit.with_connect_unix" helper function lib/nbdkit: add the "Nbdkit.get_disk_allocated" helper function lib/create_ovf: populate "actual size" attributes again lib/Makefile.am | 2 +- lib/create_ovf.ml | 33 +++++++-------- lib/create_ovf.mli | 2 +- lib/nbdkit.ml | 42 ++++++++++++++++++++ lib/nbdkit.mli | 18 +++++++++ output/output_rhv.ml | 2 +- output/output_rhv_upload.ml | 2 +- output/output_vdsm.ml | 1 + tests/test-v2v-o-rhv.ovf.expected | 4 +- tests/test-v2v-o-vdsm-options.ovf.expected | 4 +- 10 files changed, 84 insertions(+), 26 deletions(-) base-commit: bb0e698360470cb4ff5992e8e01a3165f56fe41e -- 2.19.1.3.g30247aa5d201
Connecting to an NBD server temporarily, for a "one-shot" operation, is quite similar to "Std_utils.with_open_in" and "Std_utils.with_open_out", as there are cleanup operations regardless of whether the "one-shot" operation completes successfully or throws an exception. Introduce the "Nbdkit.with_connect_unix" function, which takes a Unix domain socket pathname, a list of metadata contexts to request from the NBD server, and calls a function with the live NBD server connection. Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2027598 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- lib/Makefile.am | 2 +- lib/nbdkit.mli | 10 ++++++++++ lib/nbdkit.ml | 12 ++++++++++++ 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/lib/Makefile.am b/lib/Makefile.am index c274b9ecf6c7..1fab25b326f5 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -80,7 +80,7 @@ BOBJECTS = config.cmo $(SOURCES_ML:.ml=.cmo) XOBJECTS = $(BOBJECTS:.cmo=.cmx) OCAMLPACKAGES = \ - -package str,unix \ + -package str,unix,nbd \ -I $(builddir) \ -I $(top_builddir)/common/mlgettext \ -I $(top_builddir)/common/mlpcre \ diff --git a/lib/nbdkit.mli b/lib/nbdkit.mli index ae19295186ed..825755e61dbe 100644 --- a/lib/nbdkit.mli +++ b/lib/nbdkit.mli @@ -107,3 +107,13 @@ val run_unix : ?socket:string -> cmd -> string * int The --exit-with-parent, --foreground, --pidfile, --newstyle and --unix flags are added automatically. Other flags are set as in the {!cmd} struct. *) + +val with_connect_unix : socket:string -> + meta_contexts:string list -> + f:(NBD.t -> 'a) -> + 'a +(** [with_connect_unix socket meta_contexts f] calls function [f] with the NBD + server at Unix domain socket [socket] connected, and the metadata contexts + in [meta_contexts] requested (each of which is not necessarily supported by + the server though). The connection is torn down either on normal return or + if the function [f] throws an exception. *) diff --git a/lib/nbdkit.ml b/lib/nbdkit.ml index 8d0bfdf9cff1..43e0fc5c728a 100644 --- a/lib/nbdkit.ml +++ b/lib/nbdkit.ml @@ -217,3 +217,15 @@ If the messages above are not sufficient to diagnose the problem then add the chmod socket 0o777; socket, pid + +let with_connect_unix ~socket ~meta_contexts ~f = + let nbd = NBD.create () in + protect + ~f:(fun () -> + List.iter (NBD.add_meta_context nbd) meta_contexts; + NBD.connect_unix nbd socket; + protect + ~f:(fun () -> f nbd) + ~finally:(fun () -> NBD.shutdown nbd) + ) + ~finally:(fun () -> NBD.close nbd) -- 2.19.1.3.g30247aa5d201
On Wed, Dec 08, 2021 at 01:20:48PM +0100, Laszlo Ersek wrote:
Connecting to an NBD server temporarily, for a "one-shot" operation, is quite similar to "Std_utils.with_open_in" and "Std_utils.with_open_out", as there are cleanup operations regardless of whether the "one-shot" operation completes successfully or throws an exception.
Introduce the "Nbdkit.with_connect_unix" function, which takes a Unix domain socket pathname, a list of metadata contexts to request from the NBD server, and calls a function with the live NBD server connection.
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2027598 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- lib/Makefile.am | 2 +- lib/nbdkit.mli | 10 ++++++++++ lib/nbdkit.ml | 12 ++++++++++++ 3 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/lib/Makefile.am b/lib/Makefile.am index c274b9ecf6c7..1fab25b326f5 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -80,7 +80,7 @@ BOBJECTS = config.cmo $(SOURCES_ML:.ml=.cmo) XOBJECTS = $(BOBJECTS:.cmo=.cmx)
OCAMLPACKAGES = \ - -package str,unix \ + -package str,unix,nbd \ -I $(builddir) \ -I $(top_builddir)/common/mlgettext \ -I $(top_builddir)/common/mlpcre \ diff --git a/lib/nbdkit.mli b/lib/nbdkit.mli index ae19295186ed..825755e61dbe 100644 --- a/lib/nbdkit.mli +++ b/lib/nbdkit.mli @@ -107,3 +107,13 @@ val run_unix : ?socket:string -> cmd -> string * int The --exit-with-parent, --foreground, --pidfile, --newstyle and --unix flags are added automatically. Other flags are set as in the {!cmd} struct. *) + +val with_connect_unix : socket:string -> + meta_contexts:string list -> + f:(NBD.t -> 'a) -> + 'a +(** [with_connect_unix socket meta_contexts f] calls function [f] with the NBD + server at Unix domain socket [socket] connected, and the metadata contexts + in [meta_contexts] requested (each of which is not necessarily supported by + the server though). The connection is torn down either on normal return or + if the function [f] throws an exception. *)
Interesting choice of module for this. nbdkit is a server. I would have put it into lib/utils.mli or created a new module. The function itself looks fine although personally I might have abstracted the meta_context function to be a "do anything between creation and connection" (let's call that "preparation"). What do you think about: val with_connect_unix : socket:string -> ?prepare:(NBD.t -> unit) -> f:(NBD.t -> 'a) -> 'a I think you would have to replace the List.iter line: ...
+ ~f:(fun () ->
(match prepare with Some pf -> pf nbd | None -> ());
+ NBD.connect_unix nbd socket; + protect + ~f:(fun () -> f nbd) + ~finally:(fun () -> NBD.shutdown nbd) + ) + ~finally:(fun () -> NBD.close nbd)
It would be called like this: let prepare nbd = NBD.add_meta_context nbd "base:allocation" in with_connect_unix socket ~prepare ( fun () -> (* the code *) ) Hmm, maybe this is getting more complicated :-( Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
On 12/08/21 16:18, Richard W.M. Jones wrote:
On Wed, Dec 08, 2021 at 01:20:48PM +0100, Laszlo Ersek wrote:
Connecting to an NBD server temporarily, for a "one-shot" operation, is quite similar to "Std_utils.with_open_in" and "Std_utils.with_open_out", as there are cleanup operations regardless of whether the "one-shot" operation completes successfully or throws an exception.
Introduce the "Nbdkit.with_connect_unix" function, which takes a Unix domain socket pathname, a list of metadata contexts to request from the NBD server, and calls a function with the live NBD server connection.
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2027598 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- lib/Makefile.am | 2 +- lib/nbdkit.mli | 10 ++++++++++ lib/nbdkit.ml | 12 ++++++++++++ 3 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/lib/Makefile.am b/lib/Makefile.am index c274b9ecf6c7..1fab25b326f5 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -80,7 +80,7 @@ BOBJECTS = config.cmo $(SOURCES_ML:.ml=.cmo) XOBJECTS = $(BOBJECTS:.cmo=.cmx)
OCAMLPACKAGES = \ - -package str,unix \ + -package str,unix,nbd \ -I $(builddir) \ -I $(top_builddir)/common/mlgettext \ -I $(top_builddir)/common/mlpcre \ diff --git a/lib/nbdkit.mli b/lib/nbdkit.mli index ae19295186ed..825755e61dbe 100644 --- a/lib/nbdkit.mli +++ b/lib/nbdkit.mli @@ -107,3 +107,13 @@ val run_unix : ?socket:string -> cmd -> string * int The --exit-with-parent, --foreground, --pidfile, --newstyle and --unix flags are added automatically. Other flags are set as in the {!cmd} struct. *) + +val with_connect_unix : socket:string -> + meta_contexts:string list -> + f:(NBD.t -> 'a) -> + 'a +(** [with_connect_unix socket meta_contexts f] calls function [f] with the NBD + server at Unix domain socket [socket] connected, and the metadata contexts + in [meta_contexts] requested (each of which is not necessarily supported by + the server though). The connection is torn down either on normal return or + if the function [f] throws an exception. *)
Interesting choice of module for this. nbdkit is a server. I would have put it into lib/utils.mli or created a new module.
Yes, I was certainly lost as to what module to pick :) I found "run_unix" in this one, and thought that a wrapper for "connect_unix" (from the OCaml binding of libnbd) belonged here. I can add it to "utils" if you think that's a good fit. (I feel that creating a new module just for this is overkill, but I could be wrong.)
The function itself looks fine although personally I might have abstracted the meta_context function to be a "do anything between creation and connection" (let's call that "preparation"). What do you think about:
val with_connect_unix : socket:string -> ?prepare:(NBD.t -> unit) -> f:(NBD.t -> 'a) -> 'a
I think you would have to replace the List.iter line:
...
+ ~f:(fun () ->
(match prepare with Some pf -> pf nbd | None -> ());
+ NBD.connect_unix nbd socket; + protect + ~f:(fun () -> f nbd) + ~finally:(fun () -> NBD.shutdown nbd) + ) + ~finally:(fun () -> NBD.close nbd)
It would be called like this:
let prepare nbd = NBD.add_meta_context nbd "base:allocation" in with_connect_unix socket ~prepare ( fun () -> (* the code *) )
Hmm, maybe this is getting more complicated :-(
Well my concern is the following; let me paste the full function again: let with_connect_unix ~socket ~meta_contexts ~f = let nbd = NBD.create () in protect ~f:(fun () -> List.iter (NBD.add_meta_context nbd) meta_contexts; NBD.connect_unix nbd socket; protect ~f:(fun () -> f nbd) ~finally:(fun () -> NBD.shutdown nbd) ) ~finally:(fun () -> NBD.close nbd) "NBD.close", which mirrors "NBD.create", suffices for undoing "NBD.add_meta_context" as well. But, if we turn "NBD.add_meta_context" into a generic "do anything between NBD.create and NBD.connect_unix", I don't know if "NBD.close" can still satisfy the teardown role. In that case, we might have to accept another "rollback" function, and *that* I do find too complicated. FWIW, the current code does make a similar assumption about "f", and "NBD.shutdown". However, I thought that that was OK (and didn't need spelling out): namely, once "NBD.connect_unix" completes, any valid operation on the connection is considered... well, valid (or "expected"), and "NBD.shutdown" is considered appropriate to finish *any* such sequence of operations. I don't have the same confidence in a generic "prepare" being undone by just NBD.close. Thanks Laszlo
On Thu, Dec 09, 2021 at 11:47:12AM +0100, Laszlo Ersek wrote:
On 12/08/21 16:18, Richard W.M. Jones wrote:
On Wed, Dec 08, 2021 at 01:20:48PM +0100, Laszlo Ersek wrote:
Connecting to an NBD server temporarily, for a "one-shot" operation, is quite similar to "Std_utils.with_open_in" and "Std_utils.with_open_out", as there are cleanup operations regardless of whether the "one-shot" operation completes successfully or throws an exception.
Introduce the "Nbdkit.with_connect_unix" function, which takes a Unix domain socket pathname, a list of metadata contexts to request from the NBD server, and calls a function with the live NBD server connection.
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2027598 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- lib/Makefile.am | 2 +- lib/nbdkit.mli | 10 ++++++++++ lib/nbdkit.ml | 12 ++++++++++++ 3 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/lib/Makefile.am b/lib/Makefile.am index c274b9ecf6c7..1fab25b326f5 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -80,7 +80,7 @@ BOBJECTS = config.cmo $(SOURCES_ML:.ml=.cmo) XOBJECTS = $(BOBJECTS:.cmo=.cmx)
OCAMLPACKAGES = \ - -package str,unix \ + -package str,unix,nbd \ -I $(builddir) \ -I $(top_builddir)/common/mlgettext \ -I $(top_builddir)/common/mlpcre \ diff --git a/lib/nbdkit.mli b/lib/nbdkit.mli index ae19295186ed..825755e61dbe 100644 --- a/lib/nbdkit.mli +++ b/lib/nbdkit.mli @@ -107,3 +107,13 @@ val run_unix : ?socket:string -> cmd -> string * int The --exit-with-parent, --foreground, --pidfile, --newstyle and --unix flags are added automatically. Other flags are set as in the {!cmd} struct. *) + +val with_connect_unix : socket:string -> + meta_contexts:string list -> + f:(NBD.t -> 'a) -> + 'a +(** [with_connect_unix socket meta_contexts f] calls function [f] with the NBD + server at Unix domain socket [socket] connected, and the metadata contexts + in [meta_contexts] requested (each of which is not necessarily supported by + the server though). The connection is torn down either on normal return or + if the function [f] throws an exception. *)
Interesting choice of module for this. nbdkit is a server. I would have put it into lib/utils.mli or created a new module.
Yes, I was certainly lost as to what module to pick :) I found "run_unix" in this one, and thought that a wrapper for "connect_unix" (from the OCaml binding of libnbd) belonged here.
I can add it to "utils" if you think that's a good fit. (I feel that creating a new module just for this is overkill, but I could be wrong.)
Go with Utils. It's a dumping ground for random functions used in virt-v2v.
The function itself looks fine although personally I might have abstracted the meta_context function to be a "do anything between creation and connection" (let's call that "preparation"). What do you think about:
val with_connect_unix : socket:string -> ?prepare:(NBD.t -> unit) -> f:(NBD.t -> 'a) -> 'a
I think you would have to replace the List.iter line:
...
+ ~f:(fun () ->
(match prepare with Some pf -> pf nbd | None -> ());
+ NBD.connect_unix nbd socket; + protect + ~f:(fun () -> f nbd) + ~finally:(fun () -> NBD.shutdown nbd) + ) + ~finally:(fun () -> NBD.close nbd)
It would be called like this:
let prepare nbd = NBD.add_meta_context nbd "base:allocation" in with_connect_unix socket ~prepare ( fun () -> (* the code *) )
Hmm, maybe this is getting more complicated :-(
Well my concern is the following; let me paste the full function again:
let with_connect_unix ~socket ~meta_contexts ~f = let nbd = NBD.create () in protect ~f:(fun () -> List.iter (NBD.add_meta_context nbd) meta_contexts; NBD.connect_unix nbd socket; protect ~f:(fun () -> f nbd) ~finally:(fun () -> NBD.shutdown nbd) ) ~finally:(fun () -> NBD.close nbd)
"NBD.close", which mirrors "NBD.create", suffices for undoing "NBD.add_meta_context" as well.
But, if we turn "NBD.add_meta_context" into a generic "do anything between NBD.create and NBD.connect_unix", I don't know if "NBD.close" can still satisfy the teardown role. In that case, we might have to accept another "rollback" function, and *that* I do find too complicated.
FWIW, the current code does make a similar assumption about "f", and "NBD.shutdown". However, I thought that that was OK (and didn't need spelling out): namely, once "NBD.connect_unix" completes, any valid operation on the connection is considered... well, valid (or "expected"), and "NBD.shutdown" is considered appropriate to finish *any* such sequence of operations. I don't have the same confidence in a generic "prepare" being undone by just NBD.close.
Sure, let's go with the function as originally written, I think I was trying to over-abstract things. We can always change it later if we need to. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW
Add the "Nbdkit.get_disk_allocated" helper function, which calculates the number of allocated bytes in an output disk image, through a corresponding NBD server connection. Original "NBD.block_status" summation example code by Rich Jones (thanks!), chunking, metadata context simplification, and virt-v2v integration by myself. Chunking added because "NBD.block_status" is not 64-bit clean just yet (Eric Blake is working on it). Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2027598 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- lib/nbdkit.mli | 8 ++++++ lib/nbdkit.ml | 30 ++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/lib/nbdkit.mli b/lib/nbdkit.mli index 825755e61dbe..03107dab109d 100644 --- a/lib/nbdkit.mli +++ b/lib/nbdkit.mli @@ -117,3 +117,11 @@ val with_connect_unix : socket:string -> in [meta_contexts] requested (each of which is not necessarily supported by the server though). The connection is torn down either on normal return or if the function [f] throws an exception. *) + +val get_disk_allocated : dir:string -> disknr:int -> int64 option +(** Callable only in the finalization step. [get_disk_allocated dir disknr] + examines output disk [disknr] through the corresponding NBD server socket + that resides in [dir]. Returns the number of bytes allocated in the disk + image, according to the "base:allocation" metadata context. If the context + is not supported by the NBD server behind the socket, the function returns + None. *) diff --git a/lib/nbdkit.ml b/lib/nbdkit.ml index 43e0fc5c728a..e142c210c66f 100644 --- a/lib/nbdkit.ml +++ b/lib/nbdkit.ml @@ -229,3 +229,33 @@ let with_connect_unix ~socket ~meta_contexts ~f = ~finally:(fun () -> NBD.shutdown nbd) ) ~finally:(fun () -> NBD.close nbd) + +let get_disk_allocated ~dir ~disknr = + let socket = sprintf "%s/out%d" dir disknr + and alloc_ctx = "base:allocation" in + with_connect_unix ~socket ~meta_contexts:[alloc_ctx] + ~f:(fun nbd -> + if NBD.can_meta_context nbd alloc_ctx then ( + (* Get the list of extents, using a 2GiB chunk size as hint. *) + let size = NBD.get_size nbd + and allocated = ref 0_L + and fetch_offset = ref 0_L in + while !fetch_offset < size do + let remaining = size -^ !fetch_offset in + let fetch_size = min 0x8000_0000_L remaining in + NBD.block_status nbd fetch_size !fetch_offset + (fun ctx offset entries err -> + assert (ctx = alloc_ctx); + for i = 0 to Array.length entries / 2 - 1 do + let len = Int64.of_int32 entries.(i * 2) + and typ = entries.(i * 2 + 1) in + if Int32.logand typ 1_l = 0_l then + allocated := !allocated +^ len; + fetch_offset := !fetch_offset +^ len + done; + 0 + ) + done; + Some !allocated + ) else None + ) -- 2.19.1.3.g30247aa5d201
On Wed, Dec 08, 2021 at 01:20:49PM +0100, Laszlo Ersek wrote:
Add the "Nbdkit.get_disk_allocated" helper function, which calculates the number of allocated bytes in an output disk image, through a corresponding NBD server connection.
Original "NBD.block_status" summation example code by Rich Jones (thanks!), chunking, metadata context simplification, and virt-v2v integration by myself. Chunking added because "NBD.block_status" is not 64-bit clean just yet (Eric Blake is working on it).
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2027598 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- lib/nbdkit.mli | 8 ++++++ lib/nbdkit.ml | 30 ++++++++++++++++++++ 2 files changed, 38 insertions(+)
diff --git a/lib/nbdkit.mli b/lib/nbdkit.mli index 825755e61dbe..03107dab109d 100644 --- a/lib/nbdkit.mli +++ b/lib/nbdkit.mli @@ -117,3 +117,11 @@ val with_connect_unix : socket:string -> in [meta_contexts] requested (each of which is not necessarily supported by the server though). The connection is torn down either on normal return or if the function [f] throws an exception. *) + +val get_disk_allocated : dir:string -> disknr:int -> int64 option +(** Callable only in the finalization step. [get_disk_allocated dir disknr] + examines output disk [disknr] through the corresponding NBD server socket + that resides in [dir]. Returns the number of bytes allocated in the disk + image, according to the "base:allocation" metadata context. If the context + is not supported by the NBD server behind the socket, the function returns + None. *)
This really is the right function, wrong module. Since it only applies to output modules, the logical place is in output/output.ml, where there are already a bunch of random helper functions.
diff --git a/lib/nbdkit.ml b/lib/nbdkit.ml index 43e0fc5c728a..e142c210c66f 100644 --- a/lib/nbdkit.ml +++ b/lib/nbdkit.ml @@ -229,3 +229,33 @@ let with_connect_unix ~socket ~meta_contexts ~f = ~finally:(fun () -> NBD.shutdown nbd) ) ~finally:(fun () -> NBD.close nbd) + +let get_disk_allocated ~dir ~disknr = + let socket = sprintf "%s/out%d" dir disknr + and alloc_ctx = "base:allocation" in + with_connect_unix ~socket ~meta_contexts:[alloc_ctx] + ~f:(fun nbd -> + if NBD.can_meta_context nbd alloc_ctx then ( + (* Get the list of extents, using a 2GiB chunk size as hint. *) + let size = NBD.get_size nbd + and allocated = ref 0_L + and fetch_offset = ref 0_L in + while !fetch_offset < size do + let remaining = size -^ !fetch_offset in + let fetch_size = min 0x8000_0000_L remaining in + NBD.block_status nbd fetch_size !fetch_offset + (fun ctx offset entries err -> + assert (ctx = alloc_ctx); + for i = 0 to Array.length entries / 2 - 1 do + let len = Int64.of_int32 entries.(i * 2) + and typ = entries.(i * 2 + 1) in + if Int32.logand typ 1_l = 0_l then + allocated := !allocated +^ len; + fetch_offset := !fetch_offset +^ len + done; + 0 + ) + done; + Some !allocated + ) else None + )
But the function itself is fine. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW
On 12/08/21 16:21, Richard W.M. Jones wrote:
On Wed, Dec 08, 2021 at 01:20:49PM +0100, Laszlo Ersek wrote:
Add the "Nbdkit.get_disk_allocated" helper function, which calculates the number of allocated bytes in an output disk image, through a corresponding NBD server connection.
Original "NBD.block_status" summation example code by Rich Jones (thanks!), chunking, metadata context simplification, and virt-v2v integration by myself. Chunking added because "NBD.block_status" is not 64-bit clean just yet (Eric Blake is working on it).
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2027598 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- lib/nbdkit.mli | 8 ++++++ lib/nbdkit.ml | 30 ++++++++++++++++++++ 2 files changed, 38 insertions(+)
diff --git a/lib/nbdkit.mli b/lib/nbdkit.mli index 825755e61dbe..03107dab109d 100644 --- a/lib/nbdkit.mli +++ b/lib/nbdkit.mli @@ -117,3 +117,11 @@ val with_connect_unix : socket:string -> in [meta_contexts] requested (each of which is not necessarily supported by the server though). The connection is torn down either on normal return or if the function [f] throws an exception. *) + +val get_disk_allocated : dir:string -> disknr:int -> int64 option +(** Callable only in the finalization step. [get_disk_allocated dir disknr] + examines output disk [disknr] through the corresponding NBD server socket + that resides in [dir]. Returns the number of bytes allocated in the disk + image, according to the "base:allocation" metadata context. If the context + is not supported by the NBD server behind the socket, the function returns + None. *)
This really is the right function, wrong module.
Since it only applies to output modules, the logical place is in output/output.ml, where there are already a bunch of random helper functions.
That's precisely where I put it first (given that we have "get_disks" there too!). However, I proved simply unable to *call* the function, from "lib/create_ovf.ml", in the next patch! I tried "Output.get_disk_allocated", and I got a weird compilation failure, even though the function was properly declared in "output.mli". At that point, I couldn't decide if I was missing something about the tricky "first class modules" thing, or if it was really a layering violation to call an Output function from a *lower level* utility function (such as "create_ovf" in "lib/create_ovf.ml"). I briefly considered that, whoever called create_ovf from the top level, could *pass in* Output.get_disk_allocated to it, as a callback function. But that seemed so horrendously ugly (and uncalled-for) that I dropped the idea. Yet another idea was to collect the actual sizes for all the output disks in one go, separately, then pass in that list, to create_ovf. Then in create_ovf, I would replace: (* Iterate over the disks, adding them to the OVF document. *) List.iteri ( fun i (size, image_uuid, vol_uuid) -> ... ) (List.combine3 sizes image_uuids vol_uuids) with "combine4", adding a fourth component (the actual size) to the tuple parameter of the nameless iterator function. But that idea looked too complex as well. Halp! :)
diff --git a/lib/nbdkit.ml b/lib/nbdkit.ml index 43e0fc5c728a..e142c210c66f 100644 --- a/lib/nbdkit.ml +++ b/lib/nbdkit.ml @@ -229,3 +229,33 @@ let with_connect_unix ~socket ~meta_contexts ~f = ~finally:(fun () -> NBD.shutdown nbd) ) ~finally:(fun () -> NBD.close nbd) + +let get_disk_allocated ~dir ~disknr = + let socket = sprintf "%s/out%d" dir disknr + and alloc_ctx = "base:allocation" in + with_connect_unix ~socket ~meta_contexts:[alloc_ctx] + ~f:(fun nbd -> + if NBD.can_meta_context nbd alloc_ctx then ( + (* Get the list of extents, using a 2GiB chunk size as hint. *) + let size = NBD.get_size nbd + and allocated = ref 0_L + and fetch_offset = ref 0_L in + while !fetch_offset < size do + let remaining = size -^ !fetch_offset in + let fetch_size = min 0x8000_0000_L remaining in + NBD.block_status nbd fetch_size !fetch_offset + (fun ctx offset entries err -> + assert (ctx = alloc_ctx); + for i = 0 to Array.length entries / 2 - 1 do + let len = Int64.of_int32 entries.(i * 2) + and typ = entries.(i * 2 + 1) in + if Int32.logand typ 1_l = 0_l then + allocated := !allocated +^ len; + fetch_offset := !fetch_offset +^ len + done; + 0 + ) + done; + Some !allocated + ) else None + )
But the function itself is fine.
Thanks! Laszlo
On Thu, Dec 09, 2021 at 11:55:49AM +0100, Laszlo Ersek wrote:
On 12/08/21 16:21, Richard W.M. Jones wrote:
On Wed, Dec 08, 2021 at 01:20:49PM +0100, Laszlo Ersek wrote:
Add the "Nbdkit.get_disk_allocated" helper function, which calculates the number of allocated bytes in an output disk image, through a corresponding NBD server connection.
Original "NBD.block_status" summation example code by Rich Jones (thanks!), chunking, metadata context simplification, and virt-v2v integration by myself. Chunking added because "NBD.block_status" is not 64-bit clean just yet (Eric Blake is working on it).
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2027598 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- lib/nbdkit.mli | 8 ++++++ lib/nbdkit.ml | 30 ++++++++++++++++++++ 2 files changed, 38 insertions(+)
diff --git a/lib/nbdkit.mli b/lib/nbdkit.mli index 825755e61dbe..03107dab109d 100644 --- a/lib/nbdkit.mli +++ b/lib/nbdkit.mli @@ -117,3 +117,11 @@ val with_connect_unix : socket:string -> in [meta_contexts] requested (each of which is not necessarily supported by the server though). The connection is torn down either on normal return or if the function [f] throws an exception. *) + +val get_disk_allocated : dir:string -> disknr:int -> int64 option +(** Callable only in the finalization step. [get_disk_allocated dir disknr] + examines output disk [disknr] through the corresponding NBD server socket + that resides in [dir]. Returns the number of bytes allocated in the disk + image, according to the "base:allocation" metadata context. If the context + is not supported by the NBD server behind the socket, the function returns + None. *)
This really is the right function, wrong module.
Since it only applies to output modules, the logical place is in output/output.ml, where there are already a bunch of random helper functions.
That's precisely where I put it first (given that we have "get_disks" there too!). However, I proved simply unable to *call* the function, from "lib/create_ovf.ml", in the next patch!
Oh indeed - I didn't spot it was called from lib/
I tried "Output.get_disk_allocated", and I got a weird compilation failure, even though the function was properly declared in "output.mli".
Yes you cannot just call randomly between modules (as in, say, C) because OCaml enforces a strict ordering of top level values when the program is loaded. If you could call randomly then you might call a function before any top level values it needs have been initialized, and that would cause undefined behaviour. In this case just put it in lib/utils.ml and it'll be fine.
At that point, I couldn't decide if I was missing something about the tricky "first class modules" thing, or if it was really a layering violation to call an Output function from a *lower level* utility function (such as "create_ovf" in "lib/create_ovf.ml").
Yes it's a layering thing. [...] Other suggestions look complicated. Best to put it in lib/utils.ml We can always move things around later if we discover a cleaner way. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
Commit 255722cbf39a ("v2v: Modular virt-v2v", 2021-09-07) removed the following attributes from the OVF output: - ovf:Envelope/References/File/@ovf:size - ovf:Envelope/Section[@xsi:type='ovf:DiskSection_Type']/Disk/@ovf:actual_size Unfortunately, ovirt-engine considers the second one mandatory; without it, ovirt-engine refuses to import the OVF. Restore both attributes, using the utility functions added to the Nbdkit module previously. (If we do not have the information necessary to fill in @ovf:actual_size, we still have to generate the attribute, only with empty contents. Ovirt-engine does cope with that.) Fixes: 255722cbf39afc0b012e2ac00d16fa6ba2f8c21f Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2027598 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- lib/create_ovf.mli | 2 +- lib/create_ovf.ml | 33 +++++++++----------- output/output_rhv.ml | 2 +- output/output_rhv_upload.ml | 2 +- output/output_vdsm.ml | 1 + tests/test-v2v-o-rhv.ovf.expected | 4 +-- tests/test-v2v-o-vdsm-options.ovf.expected | 4 +-- 7 files changed, 23 insertions(+), 25 deletions(-) diff --git a/lib/create_ovf.mli b/lib/create_ovf.mli index 701152d93dd2..0d1cc5a9311a 100644 --- a/lib/create_ovf.mli +++ b/lib/create_ovf.mli @@ -46,7 +46,7 @@ val ovf_flavour_to_string : ovf_flavour -> string val create_ovf : Types.source -> Types.inspect -> Types.target_meta -> int64 list -> Types.output_allocation -> string -> string -> string list -> - string list -> string -> ovf_flavour -> DOM.doc + string list -> string -> string -> ovf_flavour -> DOM.doc (** Create the OVF file. Actually a {!DOM} document is created, not a file. It can be written diff --git a/lib/create_ovf.ml b/lib/create_ovf.ml index c9f14635363e..7bc15b74662a 100644 --- a/lib/create_ovf.ml +++ b/lib/create_ovf.ml @@ -531,7 +531,7 @@ let rec create_ovf source inspect { output_name; guestcaps; target_firmware; target_nics } sizes output_alloc output_format - sd_uuid image_uuids vol_uuids vm_uuid ovf_flavour = + sd_uuid image_uuids vol_uuids dir vm_uuid ovf_flavour = assert (List.length sizes = List.length vol_uuids); let memsize_mb = source.s_memory /^ 1024L /^ 1024L in @@ -745,7 +745,7 @@ let rec create_ovf source inspect (* Add disks to the OVF XML. *) add_disks sizes guestcaps output_alloc output_format - sd_uuid image_uuids vol_uuids ovf_flavour ovf; + sd_uuid image_uuids vol_uuids dir ovf_flavour ovf; (* Old virt-v2v ignored removable media. XXX *) @@ -791,7 +791,7 @@ and get_flavoured_section ovf ovirt_path rhv_path rhv_path_attr = function (* This modifies the OVF DOM, adding a section for each disk. *) and add_disks sizes guestcaps output_alloc output_format - sd_uuid image_uuids vol_uuids ovf_flavour ovf = + sd_uuid image_uuids vol_uuids dir ovf_flavour ovf = let references = let nodes = path_to_nodes ovf ["ovf:Envelope"; "References"] in match nodes with @@ -839,13 +839,7 @@ and add_disks sizes guestcaps output_alloc output_format b /^ 1073741824L in let size_gb = bytes_to_gb size in - (* virt-v2v 1.4x used to try to collect the actual size of the - * sparse disk. It would be possible to get this information - * accurately now by reading the extent map of the output disk - * (this function is called during finalization), but we don't - * yet do that. XXX - *) - let actual_size_gb = None in + let actual_size = Nbdkit.get_disk_allocated ~dir ~disknr:i in let format_for_rhv = match output_format with @@ -867,13 +861,11 @@ and add_disks sizes guestcaps output_alloc output_format "ovf:id", vol_uuid; "ovf:description", generated_by; ] in - (* See note above about actual_size_gb - (match t.target_overlay.ov_stats.target_actual_size with + (match actual_size with | None -> () | Some actual_size -> List.push_back attrs ("ovf:size", Int64.to_string actual_size) ); - *) e "File" !attrs [] in append_child disk references; @@ -899,11 +891,16 @@ and add_disks sizes guestcaps output_alloc output_format "ovf:disk-type", "System"; (* RHBZ#744538 *) "ovf:boot", if is_bootable_drive then "True" else "False"; ] in - (match actual_size_gb with - | None -> () - | Some actual_size_gb -> - List.push_back attrs ("ovf:actual_size", Int64.to_string actual_size_gb) - ); + (* Ovirt-engine considers the "ovf:actual_size" attribute mandatory. If + * we don't know the actual size, we must create the attribute with + * empty contents. + *) + List.push_back attrs + ("ovf:actual_size", + match actual_size with + | None -> "" + | Some actual_size -> Int64.to_string (bytes_to_gb actual_size) + ); e "Disk" !attrs [] in append_child disk disk_section; diff --git a/output/output_rhv.ml b/output/output_rhv.ml index 4550dc4f306f..b902a7ee4619 100644 --- a/output/output_rhv.ml +++ b/output/output_rhv.ml @@ -183,7 +183,7 @@ and rhv_finalize dir source inspect target_meta (* Create the metadata. *) let ovf = Create_ovf.create_ovf source inspect target_meta sizes - output_alloc output_format esd_uuid image_uuids vol_uuids vm_uuid + output_alloc output_format esd_uuid image_uuids vol_uuids dir vm_uuid Create_ovf.RHVExportStorageDomain in (* Write it to the metadata file. *) diff --git a/output/output_rhv_upload.ml b/output/output_rhv_upload.ml index 64b88b67793c..89ff350be567 100644 --- a/output/output_rhv_upload.ml +++ b/output/output_rhv_upload.ml @@ -420,7 +420,7 @@ and rhv_upload_finalize dir source inspect target_meta (* Create the metadata. *) let ovf = Create_ovf.create_ovf source inspect target_meta disk_sizes - Sparse output_format sd_uuid disk_uuids vol_uuids vm_uuid + Sparse output_format sd_uuid disk_uuids vol_uuids dir vm_uuid OVirt in let ovf = DOM.doc_to_string ovf in diff --git a/output/output_vdsm.ml b/output/output_vdsm.ml index c9b552805ccd..ce0d5b5e4b2a 100644 --- a/output/output_vdsm.ml +++ b/output/output_vdsm.ml @@ -201,6 +201,7 @@ and vdsm_finalize dir source inspect target_meta output_alloc output_format dd_uuid image_uuids vol_uuids + dir vm_uuid ovf_flavour in diff --git a/tests/test-v2v-o-rhv.ovf.expected b/tests/test-v2v-o-rhv.ovf.expected index 5fda41c9488e..25e492fdbabc 100644 --- a/tests/test-v2v-o-rhv.ovf.expected +++ b/tests/test-v2v-o-rhv.ovf.expected @@ -2,7 +2,7 @@ <ovf:Envelope xmlns:rasd='http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSett...' xmlns:vssd='http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_VirtualSystemSettingDa...' xmlns:xsi='http://www.w3.org/2001/XMLSchema-instance' xmlns:ovf='http://schemas.dmtf.org/ovf/envelope/1/' xmlns:ovirt='http://www.ovirt.org/ovf' ovf:version='0.9'> <!-- generated by virt-v2v --> <References> - <File ovf:href='#DISK_ID#/#VOL_ID#' ovf:id='#VOL_ID#' ovf:description='generated by virt-v2v'/> + <File ovf:href='#DISK_ID#/#VOL_ID#' ovf:id='#VOL_ID#' ovf:description='generated by virt-v2v' ovf:size='#SIZE#'/> </References> <Section xsi:type='ovf:NetworkSection_Type'> <Info>List of networks</Info> @@ -10,7 +10,7 @@ </Section> <Section xsi:type='ovf:DiskSection_Type'> <Info>List of Virtual Disks</Info> - <Disk ovf:diskId='#VOL_ID#' ovf:size='1' ovf:capacity='536870912' ovf:fileRef='#DISK_ID#/#VOL_ID#' ovf:parentRef='' ovf:vm_snapshot_id='#UUID#' ovf:volume-format='RAW' ovf:volume-type='Sparse' ovf:format='http://en.wikipedia.org/wiki/Byte' ovf:disk-interface='VirtIO' ovf:disk-type='System' ovf:boot='True'/> + <Disk ovf:diskId='#VOL_ID#' ovf:size='1' ovf:capacity='536870912' ovf:fileRef='#DISK_ID#/#VOL_ID#' ovf:parentRef='' ovf:vm_snapshot_id='#UUID#' ovf:volume-format='RAW' ovf:volume-type='Sparse' ovf:format='http://en.wikipedia.org/wiki/Byte' ovf:disk-interface='VirtIO' ovf:disk-type='System' ovf:boot='True' ovf:actual_size='1'/> </Section> <Content ovf:id='out' xsi:type='ovf:VirtualSystem_Type'> <Name>windows</Name> diff --git a/tests/test-v2v-o-vdsm-options.ovf.expected b/tests/test-v2v-o-vdsm-options.ovf.expected index 23ca180f4c2f..bd5b5e7d38ec 100644 --- a/tests/test-v2v-o-vdsm-options.ovf.expected +++ b/tests/test-v2v-o-vdsm-options.ovf.expected @@ -2,7 +2,7 @@ <ovf:Envelope xmlns:rasd='http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSett...' xmlns:vssd='http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_VirtualSystemSettingDa...' xmlns:xsi='http://www.w3.org/2001/XMLSchema-instance' xmlns:ovf='http://schemas.dmtf.org/ovf/envelope/1/' xmlns:ovirt='http://www.ovirt.org/ovf' ovf:version='0.9'> <!-- generated by virt-v2v --> <References> - <File ovf:href='VOL' ovf:id='VOL' ovf:description='generated by virt-v2v'/> + <File ovf:href='VOL' ovf:id='VOL' ovf:description='generated by virt-v2v' ovf:size='#SIZE#'/> </References> <NetworkSection> <Info>List of networks</Info> @@ -10,7 +10,7 @@ </NetworkSection> <DiskSection> <Info>List of Virtual Disks</Info> - <Disk ovf:diskId='IMAGE' ovf:size='1' ovf:capacity='536870912' ovf:fileRef='VOL' ovf:parentRef='' ovf:vm_snapshot_id='#UUID#' ovf:volume-format='COW' ovf:volume-type='Sparse' ovf:format='http://en.wikipedia.org/wiki/Byte' ovf:disk-interface='VirtIO' ovf:disk-type='System' ovf:boot='True'/> + <Disk ovf:diskId='IMAGE' ovf:size='1' ovf:capacity='536870912' ovf:fileRef='VOL' ovf:parentRef='' ovf:vm_snapshot_id='#UUID#' ovf:volume-format='COW' ovf:volume-type='Sparse' ovf:format='http://en.wikipedia.org/wiki/Byte' ovf:disk-interface='VirtIO' ovf:disk-type='System' ovf:boot='True' ovf:actual_size='1'/> </DiskSection> <VirtualSystem ovf:id='VM'> <Name>windows</Name> -- 2.19.1.3.g30247aa5d201
On Wed, Dec 08, 2021 at 01:20:50PM +0100, Laszlo Ersek wrote:
Commit 255722cbf39a ("v2v: Modular virt-v2v", 2021-09-07) removed the following attributes from the OVF output:
- ovf:Envelope/References/File/@ovf:size - ovf:Envelope/Section[@xsi:type='ovf:DiskSection_Type']/Disk/@ovf:actual_size
Unfortunately, ovirt-engine considers the second one mandatory; without it, ovirt-engine refuses to import the OVF.
Restore both attributes, using the utility functions added to the Nbdkit module previously.
(If we do not have the information necessary to fill in @ovf:actual_size, we still have to generate the attribute, only with empty contents. Ovirt-engine does cope with that.)
Fixes: 255722cbf39afc0b012e2ac00d16fa6ba2f8c21f Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2027598 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- lib/create_ovf.mli | 2 +- lib/create_ovf.ml | 33 +++++++++----------- output/output_rhv.ml | 2 +- output/output_rhv_upload.ml | 2 +- output/output_vdsm.ml | 1 + tests/test-v2v-o-rhv.ovf.expected | 4 +-- tests/test-v2v-o-vdsm-options.ovf.expected | 4 +-- 7 files changed, 23 insertions(+), 25 deletions(-)
diff --git a/lib/create_ovf.mli b/lib/create_ovf.mli index 701152d93dd2..0d1cc5a9311a 100644 --- a/lib/create_ovf.mli +++ b/lib/create_ovf.mli @@ -46,7 +46,7 @@ val ovf_flavour_to_string : ovf_flavour -> string val create_ovf : Types.source -> Types.inspect -> Types.target_meta -> int64 list -> Types.output_allocation -> string -> string -> string list -> - string list -> string -> ovf_flavour -> DOM.doc + string list -> string -> string -> ovf_flavour -> DOM.doc (** Create the OVF file.
Actually a {!DOM} document is created, not a file. It can be written diff --git a/lib/create_ovf.ml b/lib/create_ovf.ml index c9f14635363e..7bc15b74662a 100644 --- a/lib/create_ovf.ml +++ b/lib/create_ovf.ml @@ -531,7 +531,7 @@ let rec create_ovf source inspect { output_name; guestcaps; target_firmware; target_nics } sizes output_alloc output_format - sd_uuid image_uuids vol_uuids vm_uuid ovf_flavour = + sd_uuid image_uuids vol_uuids dir vm_uuid ovf_flavour = assert (List.length sizes = List.length vol_uuids);
let memsize_mb = source.s_memory /^ 1024L /^ 1024L in @@ -745,7 +745,7 @@ let rec create_ovf source inspect
(* Add disks to the OVF XML. *) add_disks sizes guestcaps output_alloc output_format - sd_uuid image_uuids vol_uuids ovf_flavour ovf; + sd_uuid image_uuids vol_uuids dir ovf_flavour ovf;
(* Old virt-v2v ignored removable media. XXX *)
@@ -791,7 +791,7 @@ and get_flavoured_section ovf ovirt_path rhv_path rhv_path_attr = function
(* This modifies the OVF DOM, adding a section for each disk. *) and add_disks sizes guestcaps output_alloc output_format - sd_uuid image_uuids vol_uuids ovf_flavour ovf = + sd_uuid image_uuids vol_uuids dir ovf_flavour ovf = let references = let nodes = path_to_nodes ovf ["ovf:Envelope"; "References"] in match nodes with @@ -839,13 +839,7 @@ and add_disks sizes guestcaps output_alloc output_format b /^ 1073741824L in let size_gb = bytes_to_gb size in - (* virt-v2v 1.4x used to try to collect the actual size of the - * sparse disk. It would be possible to get this information - * accurately now by reading the extent map of the output disk - * (this function is called during finalization), but we don't - * yet do that. XXX - *) - let actual_size_gb = None in + let actual_size = Nbdkit.get_disk_allocated ~dir ~disknr:i in
let format_for_rhv = match output_format with @@ -867,13 +861,11 @@ and add_disks sizes guestcaps output_alloc output_format "ovf:id", vol_uuid; "ovf:description", generated_by; ] in - (* See note above about actual_size_gb - (match t.target_overlay.ov_stats.target_actual_size with + (match actual_size with | None -> () | Some actual_size -> List.push_back attrs ("ovf:size", Int64.to_string actual_size) ); - *) e "File" !attrs [] in append_child disk references;
@@ -899,11 +891,16 @@ and add_disks sizes guestcaps output_alloc output_format "ovf:disk-type", "System"; (* RHBZ#744538 *) "ovf:boot", if is_bootable_drive then "True" else "False"; ] in - (match actual_size_gb with - | None -> () - | Some actual_size_gb -> - List.push_back attrs ("ovf:actual_size", Int64.to_string actual_size_gb) - ); + (* Ovirt-engine considers the "ovf:actual_size" attribute mandatory. If + * we don't know the actual size, we must create the attribute with + * empty contents. + *) + List.push_back attrs + ("ovf:actual_size", + match actual_size with + | None -> "" + | Some actual_size -> Int64.to_string (bytes_to_gb actual_size) + ); e "Disk" !attrs [] in append_child disk disk_section;
diff --git a/output/output_rhv.ml b/output/output_rhv.ml index 4550dc4f306f..b902a7ee4619 100644 --- a/output/output_rhv.ml +++ b/output/output_rhv.ml @@ -183,7 +183,7 @@ and rhv_finalize dir source inspect target_meta (* Create the metadata. *) let ovf = Create_ovf.create_ovf source inspect target_meta sizes - output_alloc output_format esd_uuid image_uuids vol_uuids vm_uuid + output_alloc output_format esd_uuid image_uuids vol_uuids dir vm_uuid Create_ovf.RHVExportStorageDomain in
(* Write it to the metadata file. *) diff --git a/output/output_rhv_upload.ml b/output/output_rhv_upload.ml index 64b88b67793c..89ff350be567 100644 --- a/output/output_rhv_upload.ml +++ b/output/output_rhv_upload.ml @@ -420,7 +420,7 @@ and rhv_upload_finalize dir source inspect target_meta (* Create the metadata. *) let ovf = Create_ovf.create_ovf source inspect target_meta disk_sizes - Sparse output_format sd_uuid disk_uuids vol_uuids vm_uuid + Sparse output_format sd_uuid disk_uuids vol_uuids dir vm_uuid OVirt in let ovf = DOM.doc_to_string ovf in
diff --git a/output/output_vdsm.ml b/output/output_vdsm.ml index c9b552805ccd..ce0d5b5e4b2a 100644 --- a/output/output_vdsm.ml +++ b/output/output_vdsm.ml @@ -201,6 +201,7 @@ and vdsm_finalize dir source inspect target_meta output_alloc output_format dd_uuid image_uuids vol_uuids + dir vm_uuid ovf_flavour in
diff --git a/tests/test-v2v-o-rhv.ovf.expected b/tests/test-v2v-o-rhv.ovf.expected index 5fda41c9488e..25e492fdbabc 100644 --- a/tests/test-v2v-o-rhv.ovf.expected +++ b/tests/test-v2v-o-rhv.ovf.expected @@ -2,7 +2,7 @@ <ovf:Envelope xmlns:rasd='http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSett...' xmlns:vssd='http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_VirtualSystemSettingDa...' xmlns:xsi='http://www.w3.org/2001/XMLSchema-instance' xmlns:ovf='http://schemas.dmtf.org/ovf/envelope/1/' xmlns:ovirt='http://www.ovirt.org/ovf' ovf:version='0.9'> <!-- generated by virt-v2v --> <References> - <File ovf:href='#DISK_ID#/#VOL_ID#' ovf:id='#VOL_ID#' ovf:description='generated by virt-v2v'/> + <File ovf:href='#DISK_ID#/#VOL_ID#' ovf:id='#VOL_ID#' ovf:description='generated by virt-v2v' ovf:size='#SIZE#'/> </References> <Section xsi:type='ovf:NetworkSection_Type'> <Info>List of networks</Info> @@ -10,7 +10,7 @@ </Section> <Section xsi:type='ovf:DiskSection_Type'> <Info>List of Virtual Disks</Info> - <Disk ovf:diskId='#VOL_ID#' ovf:size='1' ovf:capacity='536870912' ovf:fileRef='#DISK_ID#/#VOL_ID#' ovf:parentRef='' ovf:vm_snapshot_id='#UUID#' ovf:volume-format='RAW' ovf:volume-type='Sparse' ovf:format='http://en.wikipedia.org/wiki/Byte' ovf:disk-interface='VirtIO' ovf:disk-type='System' ovf:boot='True'/> + <Disk ovf:diskId='#VOL_ID#' ovf:size='1' ovf:capacity='536870912' ovf:fileRef='#DISK_ID#/#VOL_ID#' ovf:parentRef='' ovf:vm_snapshot_id='#UUID#' ovf:volume-format='RAW' ovf:volume-type='Sparse' ovf:format='http://en.wikipedia.org/wiki/Byte' ovf:disk-interface='VirtIO' ovf:disk-type='System' ovf:boot='True' ovf:actual_size='1'/> </Section> <Content ovf:id='out' xsi:type='ovf:VirtualSystem_Type'> <Name>windows</Name> diff --git a/tests/test-v2v-o-vdsm-options.ovf.expected b/tests/test-v2v-o-vdsm-options.ovf.expected index 23ca180f4c2f..bd5b5e7d38ec 100644 --- a/tests/test-v2v-o-vdsm-options.ovf.expected +++ b/tests/test-v2v-o-vdsm-options.ovf.expected @@ -2,7 +2,7 @@ <ovf:Envelope xmlns:rasd='http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSett...' xmlns:vssd='http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_VirtualSystemSettingDa...' xmlns:xsi='http://www.w3.org/2001/XMLSchema-instance' xmlns:ovf='http://schemas.dmtf.org/ovf/envelope/1/' xmlns:ovirt='http://www.ovirt.org/ovf' ovf:version='0.9'> <!-- generated by virt-v2v --> <References> - <File ovf:href='VOL' ovf:id='VOL' ovf:description='generated by virt-v2v'/> + <File ovf:href='VOL' ovf:id='VOL' ovf:description='generated by virt-v2v' ovf:size='#SIZE#'/> </References> <NetworkSection> <Info>List of networks</Info> @@ -10,7 +10,7 @@ </NetworkSection> <DiskSection> <Info>List of Virtual Disks</Info> - <Disk ovf:diskId='IMAGE' ovf:size='1' ovf:capacity='536870912' ovf:fileRef='VOL' ovf:parentRef='' ovf:vm_snapshot_id='#UUID#' ovf:volume-format='COW' ovf:volume-type='Sparse' ovf:format='http://en.wikipedia.org/wiki/Byte' ovf:disk-interface='VirtIO' ovf:disk-type='System' ovf:boot='True'/> + <Disk ovf:diskId='IMAGE' ovf:size='1' ovf:capacity='536870912' ovf:fileRef='VOL' ovf:parentRef='' ovf:vm_snapshot_id='#UUID#' ovf:volume-format='COW' ovf:volume-type='Sparse' ovf:format='http://en.wikipedia.org/wiki/Byte' ovf:disk-interface='VirtIO' ovf:disk-type='System' ovf:boot='True' ovf:actual_size='1'/> </DiskSection> <VirtualSystem ovf:id='VM'> <Name>windows</Name> -- 2.19.1.3.g30247aa5d201
This patch looks fine. 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
participants (2)
-
Laszlo Ersek -
Richard W.M. Jones