[Libguestfs] [virt-v2v PATCH 2/3] lib/nbdkit: add the "Nbdkit.get_disk_allocated" helper function

[Laszlo Ersek]

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!

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