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 <mklet...@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
signature.asc
Description: PGP signature
_______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
