On Fri, Aug 04, 2023 at 10:11:06AM -0500, Eric Blake wrote: > On Fri, Aug 04, 2023 at 10:09:14AM +0100, Richard W.M. Jones wrote: > > On Wed, Aug 02, 2023 at 08:50:22PM -0500, Eric Blake wrote: > > > The existing nbd_block_status() callback is permanently stuck with an > > > array of uint32_t pairs (each of the h->bs_count extents is > > > represented by a pair of uint32_t; we have to pass bs_count*2 as the > > > array size to the callback, and the user has to compute len/2 to > > > determine how many extents to process). Furthermore, the 32-bit > > > nature of both values adds some constraints: we cannot represent > > > extents larger than 4G, and status flags must fit in 32 bits. While > > > the "base:allocation" metacontext will never exceed 32 bits, it is not > > > hard to envision other future extension metacontexts where a 64-bit > > > status would be useful (for example, Zoned Block Devices expressing a > > > 64-bit offset[1]). > > > > > > Exposing 64-bit extents will require a new API; we now have the > > > decision of whether to copy the existing API practice of returning a > > > bare array containing h->bs_count*2 raw uint64_t (where the user > > > continues the same practice of pairing those off into extents), or > > > going with a saner idea of an array of h->bs_count structs directly > > > describing extents. Returning an array of structs has an advantage: > > > although both items in the 64-bit extent are 64-bit values over the > > > wire, they are conceptually different types (an extent length is > > > inherently bounded by 63-bit off_t limitations; while the extent flags > > > can be a full 64 bits of unsigned flags), and this difference in types > > > can be more precisely represented in non-C languages. > > > > > > This patch starts the ball rolling by adding a new arg type for use in > > > the generator, although all match statements that process args are > > > minimally changed to merely assert that we aren't using the type yet. > > > Then the next few patches can provide an implementation per language > > > binding (for ease of review), prior to a final patch that finally adds > > > the new nbd_block_status_64() API that actually takes advantage of the > > > new type. > > > > > > [1] https://zonedstorage.io/docs/linux/zbd-api > > > > > > Signed-off-by: Eric Blake <ebl...@redhat.com> > > > --- > > > > > > v4: improve commit message [Laszlo], split into several smaller > > > patches [Laszlo] > > > --- > > > generator/API.mli | 1 + > > > generator/API.ml | 12 +++++++++++- > > > generator/C.ml | 7 +++++++ > > > generator/GoLang.ml | 4 ++++ > > > generator/OCaml.ml | 4 ++++ > > > generator/Python.ml | 5 +++++ > > > 6 files changed, 32 insertions(+), 1 deletion(-) > > > > > > diff --git a/generator/API.mli b/generator/API.mli > > > index c5bba8c2..80633018 100644 > > > --- a/generator/API.mli > > > +++ b/generator/API.mli > > > @@ -52,6 +52,7 @@ and > > > | BytesPersistOut of string * string > > > | Closure of closure (** function pointer + void *opaque *) > > > | Enum of string * enum (** enum/union type, int in C *) > > > +| Extent64 of string (** extent descriptor *) > > > > In the next patch this becomes: > > > > +| Extent64 of string (** extent descriptor, struct nbd_extent in C *) > > > > but neither is really helpful if you're just looking at this line of > > code in isolation. Could we change the comment to say something like > > this (no need to update it in the next patch): > > > > | Extent64 of string (** extent descriptor, > > struct nbd_extent in C, > > pair of 64 bit ints in other languages *) > > > > as that seems much clearer to me. > > Seems reasonable. Although maybe it would be more accurate as: > > (** extent descriptor, with 63-bit size and 64-bit flags; > struct nbd_extent in C, tuple or pair in other languages *) > > to make it a bit more obvious that language bindings may give the size > component a different type than the flags?
Sounds good. 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 _______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
