[Libguestfs] [libnbd PATCH] generator: Add support for namespace constants
Eric Blake
eblake at redhat.com
Thu Jun 27 14:25:38 UTC 2019
On 6/27/19 5:07 AM, Martin Kletzander wrote:
> This just defines the namespace, its contexts and related constants and the only
> supported one is currently base:allocation. The names of the constants are not
> very future-proof, but shorter than LIBNBD_META_NS_CONTEXT_BASE_ALLOCATION or
> similar.
>
> Currently the output looks like this:
>
> /* "base" namespace */
>
> /* "base" namespace contexts */
>
> /* "base:allocation" context related constants */
>
> Separated by two empty lines from unrelated parts of the header file above and
> below.
>
> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
> ---
>
> Notes:
> Everything is up for a debate:
>
> - the names might need to be different so that they do not clash with other
> constants in the scope later on,
Yeah, could be a problem. We have until API stability freeze to change
our minds. I'm guessing we might need:
LIBNBD_CONTEXT_BASE_ALLOCATION_STATE_HOLE
LIBNBD_CONTEXT_BASE_ALLOCATION_STATE_ZERO
but yes, that's a mouthful to type. Keeping the short names
LIBNBD_STATE_HOLE for now is okay.
>
> - the fact that "base" and "base:allocation" are even defined, which might be
> useless, since listing contexts of a namespace is not exposed,
Rich still has the idea of adding a 'qemu-nbd --list' counterpart, so
defining a constant for the "base:" and "qemu:" namespaces makes sense
for that even if we can't use it now. Hmm - your patch defines
LIBNBD_NAMESPACE_BASE to "base", but in practice we'd want to pass
"base:" when querying which contexts are available in that namespace.
>
> - whether this should live in a separate (still included in libnbd.h) file,
Single file is fine by me for now; we can always split later if it gets
huge.
>
> - and more...
I'd love to have LIBNBD_CONTEXT_QEMU_DIRTY_BITMAP(foo) produce the
string "qemu:dirty-bitmap:foo". I'm not sure how to wire that in on top
of your patches, so it doesn't have to happen today, but it's food for
thought.
Here's what I'm pushing as a followup to your patch, to add more
documentation about the constants, and to use them in our testsuite:
diff --git i/generator/generator w/generator/generator
index 9d8d257..1cfcb3d 100755
--- i/generator/generator
+++ w/generator/generator
@@ -1118,7 +1118,10 @@ was actually negotiated, call
C<nbd_can_meta_context> after
connecting.
The single parameter is the name of the metadata context,
-for example C<\"base:allocation\">.
+for example C<LIBNBD_CONTEXT_BASE_ALLOCATION>.
+B<E<lt>libnbd.hE<gt>> includes defined constants beginning with
+C<LIBNBD_CONTEXT_> for some well-known contexts, but you are free
+to pass in other contexts.
Other metadata contexts are server-specific, but include
C<\"qemu:dirty-bitmap:...\"> for qemu-nbd
@@ -1293,7 +1296,10 @@ Returns true if the server supports the given
meta context
the server does not.
The single parameter is the name of the metadata context,
-for example C<\"base:allocation\">.";
+for example C<LIBNBD_CONTEXT_BASE_ALLOCATION>.
+B<E<lt>libnbd.hE<gt>> includes defined constants for well-known
+namespace contexts beginning with C<LIBNBD_CONTEXT_>, but you
+are free to pass in other contexts.";
};
"get_size", {
@@ -1551,7 +1557,9 @@ pair being the length (in bytes) of the block and
the second entry
being a status/flags field which is specific to the metadata context.
(The number of pairs passed to the function is C<nr_entries/2>.) The
NBD protocol document in the section about
-C<NBD_REPLY_TYPE_BLOCK_STATUS> describes the meaning of this array.
+C<NBD_REPLY_TYPE_BLOCK_STATUS> describes the meaning of this array;
+for contexts known to libnbd, B<E<lt>libnbd.hE<gt>> contains constants
+beginning with C<LIBNBD_STATE_> that may help decipher the values.
It is possible for the extent function to be called
more times than you expect (if the server is buggy),
@@ -2926,7 +2934,7 @@ let print_ns_ctxt ns ns_upper ctxt consts =
let print_ns ns ctxts =
let ns_upper = String.uppercase_ascii ns in
pr "/* \"%s\" namespace */\n" ns;
- pr "#define LIBNBD_NAMESPACE_%s \"%s\"\n" ns_upper ns;
+ pr "#define LIBNBD_NAMESPACE_%s \"%s:\"\n" ns_upper ns;
pr "\n";
pr "/* \"%s\" namespace contexts */\n" ns;
List.iter (
diff --git i/interop/dirty-bitmap.c w/interop/dirty-bitmap.c
index 61661fa..329fbea 100644
--- i/interop/dirty-bitmap.c
+++ w/interop/dirty-bitmap.c
@@ -30,7 +30,6 @@
static const char *unixsocket;
static const char *bitmap;
-static const char *base_allocation = "base:allocation";
struct data {
bool req_one; /* input: true if req_one was passed to request */
@@ -53,7 +52,7 @@ cb (void *opaque, const char *metacontext, uint64_t
offset,
assert (offset == 0);
assert (data->count-- > 0); /* [qemu-nbd] */
- if (strcmp (metacontext, base_allocation) == 0) {
+ if (strcmp (metacontext, LIBNBD_CONTEXT_BASE_ALLOCATION) == 0) {
assert (!data->seen_base); /* [qemu-nbd] */
data->seen_base = true;
assert (len == (data->req_one ? 2 : 8)); /* [qemu-nbd] */
@@ -62,11 +61,13 @@ cb (void *opaque, const char *metacontext, uint64_t
offset,
assert (entries[0] == 131072); assert (entries[1] == 0);
if (!data->req_one) {
/* hole|zero offset 128k size 384k */
- assert (entries[2] == 393216); assert (entries[3] == 3);
+ assert (entries[2] == 393216); assert (entries[3] ==
(LIBNBD_STATE_HOLE|
+
LIBNBD_STATE_ZERO));
/* allocated zero offset 512k size 64k */
- assert (entries[4] == 65536); assert (entries[5] == 2);
+ assert (entries[4] == 65536); assert (entries[5] ==
LIBNBD_STATE_ZERO);
/* hole|zero offset 576k size 448k */
- assert (entries[6] == 458752); assert (entries[7] == 3);
+ assert (entries[6] == 458752); assert (entries[7] ==
(LIBNBD_STATE_HOLE|
+
LIBNBD_STATE_ZERO));
}
}
else if (strcmp (metacontext, bitmap) == 0) {
@@ -117,7 +118,7 @@ main (int argc, char *argv[])
exit (EXIT_FAILURE);
}
- nbd_add_meta_context (nbd, base_allocation);
+ nbd_add_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION);
nbd_add_meta_context (nbd, bitmap);
if (nbd_connect_unix (nbd, unixsocket) == -1) {
diff --git i/tests/meta-base-allocation.c w/tests/meta-base-allocation.c
index b00aa50..a9ddbd0 100644
--- i/tests/meta-base-allocation.c
+++ w/tests/meta-base-allocation.c
@@ -56,7 +56,7 @@ main (int argc, char *argv[])
/* Negotiate metadata context "base:allocation" with the server.
* This is supported in nbdkit >= 1.12.
*/
- if (nbd_add_meta_context (nbd, "base:allocation") == -1) {
+ if (nbd_add_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION) == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
}
@@ -85,7 +85,7 @@ main (int argc, char *argv[])
exit (EXIT_FAILURE);
}
- switch (nbd_can_meta_context (nbd, "base:allocation")) {
+ switch (nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION)) {
case -1:
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
@@ -137,7 +137,7 @@ check_extent (void *data, const char *metacontext,
"nr_entries=%zu\n",
id, metacontext, offset, nr_entries);
- if (strcmp (metacontext, "base:allocation") == 0) {
+ if (strcmp (metacontext, LIBNBD_CONTEXT_BASE_ALLOCATION) == 0) {
for (i = 0; i < nr_entries; i += 2) {
printf ("\t%zu\tlength=%" PRIu32 ", status=%" PRIu32 "\n",
i, entries[i], entries[i+1]);
@@ -148,21 +148,24 @@ check_extent (void *data, const char *metacontext,
case 1:
assert (nr_entries == 10);
assert (entries[0] == 8192); assert (entries[1] == 0);
- assert (entries[2] == 8192); assert (entries[3] == 1);
- assert (entries[4] == 16384); assert (entries[5] == 3);
- assert (entries[6] == 16384); assert (entries[7] == 2);
+ assert (entries[2] == 8192); assert (entries[3] ==
LIBNBD_STATE_HOLE);
+ assert (entries[4] == 16384); assert (entries[5] ==
(LIBNBD_STATE_HOLE|
+
LIBNBD_STATE_ZERO));
+ assert (entries[6] == 16384); assert (entries[7] ==
LIBNBD_STATE_ZERO);
assert (entries[8] == 16384); assert (entries[9] == 0);
break;
case 2:
assert (nr_entries == 4);
- assert (entries[0] == 512); assert (entries[1] == 3);
- assert (entries[2] == 16384); assert (entries[3] == 2);
+ assert (entries[0] == 512); assert (entries[1] ==
(LIBNBD_STATE_HOLE|
+
LIBNBD_STATE_ZERO));
+ assert (entries[2] == 16384); assert (entries[3] ==
LIBNBD_STATE_ZERO);
break;
case 3:
assert (nr_entries == 2);
- assert (entries[0] == 512); assert (entries[1] == 3);
+ assert (entries[0] == 512); assert (entries[1] ==
(LIBNBD_STATE_HOLE|
+
LIBNBD_STATE_ZERO));
break;
default:
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20190627/fa4dbdbb/attachment.sig>
More information about the Libguestfs
mailing list