[Libguestfs] [libnbd PATCH] generator: Add support for namespace constants

[Martin Kletzander]

On Thu, Jun 27, 2019 at 09:25:38AM -0500, Eric Blake wrote:
>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.
>

I forgot to say I also went with that naming as NBD_STATE_{HOLE,ZERO} are
defined and spelled that way in the NBD protocol spec.

>>
>>     - 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.
>

That is on the protocol, but the API does not need to take the ':*' suffix.
Also it is costless to do LIBNBD_NAMESPACE_BASE ":" in the C code.

>>
>>     - 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.
>

You mean, for example:

#define LIBNBD_CONTEXT_QEMU_DIRTY_BITMAP(foo) "qemu:dirty-bitmap:" #foo

Although I'm not sure about all the corner cases, like if there can be (or need
to be) parentheses around the result.

>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:
>

Oh yes, I completely missed that.  Not only have I meant this as kind of an RFC,
but I also completely missed the documentation and tests.  So thanks a lot for
that fix.

Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20190628/dc14892b/attachment.sig>