[Libguestfs] [virt-v2v PATCH 2/3] lib/nbdkit: add the "Nbdkit.get_disk_allocated" helper function
Richard W.M. Jones
rjones at redhat.com
Thu Dec 9 11:11:59 UTC 2021
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 at 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
More information about the Libguestfs
mailing list