Eric Blake
2023-Jul-26 17:29 UTC
[Libguestfs] [libnbd PATCH 1/7] api: Expose "qemu:" meta-context constants
Qemu has a couple of documented meta-context definitions[1]; we might
as well expose constants for these namespaces.
"qemu:dirty-bitmap:NAME" is a set of namespaces for any arbitrary
dirty bitmap name; we can't define constants for every bitspace name,
but it is possible to do NBD_OPT_LIST_META_CONTEXT on
"qemu:dirty-bitmap:" to see which dirty bitmaps are available. When a
dirty bitmap is negotiated, only one bit is defined (an extent is
dirty or not). The presence of '-' and ':' in the context name
beyond
the namespace requires a new helper function.
"qemu:allocation-depth" returns an integer rather than a bitmap (0 for
unmapped, 1 for current image, 2 and beyond for number of files in the
backing chain before the data was supplied), so we can't really define
any constants for interpreting its values.
[1] https://gitlab.com/qemu-project/qemu/-/blob/master/docs/interop/nbd.txt
For libnbd.h, the generated diff is:
| --- include/libnbd.h.bak 2023-07-26 11:01:45.401328604 -0500
| +++ include/libnbd.h 2023-07-26 11:59:38.289021067 -0500
| @@ -1083,6 +1083,16 @@
| #define LIBNBD_STATE_HOLE 1
| #define LIBNBD_STATE_ZERO 2
|
| +/* "qemu" namespace */
| +#define LIBNBD_NAMESPACE_QEMU "qemu:"
| +
| +/* "qemu" namespace contexts */
| +#define LIBNBD_CONTEXT_QEMU_DIRTY_BITMAP "qemu:dirty-bitmap:"
| +#define LIBNBD_CONTEXT_QEMU_ALLOCATION_DEPTH
"qemu:allocation-depth"
| +
| +/* "qemu:dirty-bitmap:" context related constants */
| +#define LIBNBD_STATE_DIRTY 1
| +
| #ifdef __cplusplus
| }
| #endif
Signed-off-by: Eric Blake <eblake at redhat.com>
---
generator/utils.mli | 1 +
generator/API.ml | 9 ++++++---
generator/C.ml | 22 +++++++++++++++-------
generator/GoLang.ml | 3 ++-
generator/OCaml.ml | 4 ++--
generator/Python.ml | 3 ++-
generator/utils.ml | 5 +++++
interop/dirty-bitmap.c | 6 ++++--
8 files changed, 37 insertions(+), 16 deletions(-)
diff --git a/generator/utils.mli b/generator/utils.mli
index 773e705f..c4d47a34 100644
--- a/generator/utils.mli
+++ b/generator/utils.mli
@@ -46,6 +46,7 @@ val
val cspan : string -> string -> int
val quote : string -> string
val spaces : int -> string
+val macro_name : string -> string
val files_equal : string -> string -> bool
val generate_header :
diff --git a/generator/API.ml b/generator/API.ml
index 02c1260d..72c81657 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -3919,9 +3919,12 @@ let constants
let metadata_namespaces = [
"base", [ "allocation", [
- "STATE_HOLE", 1 lsl 0;
- "STATE_ZERO", 1 lsl 1;
- ] ];
+ "STATE_HOLE", 1 lsl 0;
+ "STATE_ZERO", 1 lsl 1;
+ ] ];
+ "qemu", [ "dirty-bitmap:", [ "STATE_DIRTY", 1
lsl 0; ];
+ "allocation-depth", [];
+ ];
]
let pod_of_link = function
diff --git a/generator/C.ml b/generator/C.ml
index f772117c..a2670f70 100644
--- a/generator/C.ml
+++ b/generator/C.ml
@@ -373,13 +373,18 @@ let
pr "#define LIBNBD_HAVE_NBD_%s 1\n" name_upper;
pr "\n"
-let print_ns_ctxt ns ns_upper ctxt consts - let ctxt_upper =
String.uppercase_ascii ctxt in
+let print_ns_ctxt ns ns_upper ctxt + let ctxt_macro = macro_name ctxt in
+ let ctxt_upper = String.uppercase_ascii ctxt_macro in
pr "#define LIBNBD_CONTEXT_%s_%s \"%s:%s\"\n"
- ns_upper ctxt_upper ns ctxt;
- pr "\n";
- pr "/* \"%s:%s\" context related constants */\n" ns ctxt;
- List.iter (fun (n, i) -> pr "#define LIBNBD_%-30s %d\n" n i)
consts
+ ns_upper ctxt_upper ns ctxt
+
+let print_ns_ctxt_bits ns ctxt consts + if consts <> [] then (
+ pr "\n";
+ pr "/* \"%s:%s\" context related constants */\n" ns
ctxt;
+ List.iter (fun (n, i) -> pr "#define LIBNBD_%-30s %d\n" n i)
consts
+ )
let print_ns ns ctxts let ns_upper = String.uppercase_ascii ns in
@@ -388,7 +393,10 @@ let
pr "\n";
pr "/* \"%s\" namespace contexts */\n" ns;
List.iter (
- fun (ctxt, consts) -> print_ns_ctxt ns ns_upper ctxt consts
+ fun (ctxt, _) -> print_ns_ctxt ns ns_upper ctxt
+ ) ctxts;
+ List.iter (
+ fun (ctxt, consts) -> print_ns_ctxt_bits ns ctxt consts
) ctxts;
pr "\n"
diff --git a/generator/GoLang.ml b/generator/GoLang.ml
index 50ed7226..7a7e7f4b 100644
--- a/generator/GoLang.ml
+++ b/generator/GoLang.ml
@@ -472,7 +472,8 @@ let
pr " namespace_%s = \"%s:\"\n" ns ns;
List.iter (
fun (ctxt, consts) ->
- pr " context_%s_%s = \"%s:%s\"\n" ns ctxt ns
ctxt;
+ let ctxt_macro = macro_name ctxt in
+ pr " context_%s_%s = \"%s:%s\"\n" ns
ctxt_macro ns ctxt;
List.iter (fun (n, v) ->
pr " %s uint32 = %d\n" n v
) consts
diff --git a/generator/OCaml.ml b/generator/OCaml.ml
index edb81f25..efc1af22 100644
--- a/generator/OCaml.ml
+++ b/generator/OCaml.ml
@@ -183,7 +183,7 @@ type
pr "val namespace_%s : string\n" ns;
List.iter (
fun (ctxt, consts) ->
- pr "val context_%s_%s : string\n" ns ctxt;
+ pr "val context_%s_%s : string\n" ns (macro_name ctxt);
List.iter (fun (n, _) ->
pr "val %s : int32\n" (String.lowercase_ascii n)
) consts
@@ -315,7 +315,7 @@ let
pr "let namespace_%s = \"%s:\"\n" ns ns;
List.iter (
fun (ctxt, consts) ->
- pr "let context_%s_%s = \"%s:%s\"\n" ns ctxt ns
ctxt;
+ pr "let context_%s_%s = \"%s:%s\"\n" ns
(macro_name ctxt) ns ctxt;
List.iter (fun (n, i) ->
pr "let %s = %d_l\n" (String.lowercase_ascii n) i
) consts
diff --git a/generator/Python.ml b/generator/Python.ml
index c81878de..beaeaa4c 100644
--- a/generator/Python.ml
+++ b/generator/Python.ml
@@ -745,7 +745,8 @@ let
pr "NAMESPACE_%s = \"%s:\"\n" ns_upper ns;
List.iter (
fun (ctxt, consts) ->
- let ctxt_upper = String.uppercase_ascii ctxt in
+ let ctxt_macro = macro_name ctxt in
+ let ctxt_upper = String.uppercase_ascii ctxt_macro in
pr "%s = \"%s:%s\"\n"
(sprintf "CONTEXT_%s_%s" ns_upper ctxt_upper) ns ctxt;
List.iter (fun (n, i) -> pr "%s = %d\n" n i) consts
diff --git a/generator/utils.ml b/generator/utils.ml
index e0c67d20..61cce877 100644
--- a/generator/utils.ml
+++ b/generator/utils.ml
@@ -151,6 +151,11 @@ let
let spaces n = String.make n ' '
+(* Convert s to macro name by changing '-' to '_' and eliding
':'. *)
+let macro_name s + let underscore = Str.global_replace (Str.regexp_string
"-") "_" s in
+ Str.global_replace (Str.regexp ":") "" underscore
+
(* Save the current output channel and replace it with a temporary buffer while
* running ???code???. Return the buffer.
*)
diff --git a/interop/dirty-bitmap.c b/interop/dirty-bitmap.c
index 05f6e9db..16faaed9 100644
--- a/interop/dirty-bitmap.c
+++ b/interop/dirty-bitmap.c
@@ -127,8 +127,10 @@ main (int argc, char *argv[])
nbd_extent_callback extent_callback = { .callback = cb, .user_data =
&data };
char c;
- if (argc < 3) {
- fprintf (stderr, "%s bitmap qemu-nbd [args ...]\n", argv[0]);
+ if (argc < 3 || strncmp (argv[1], LIBNBD_CONTEXT_QEMU_DIRTY_BITMAP,
+ strlen (LIBNBD_CONTEXT_QEMU_DIRTY_BITMAP)) != 0) {
+ fprintf (stderr, "%s qemu:dirty-bitmap:name qemu-nbd [args
...]\n",
+ argv[0]);
exit (EXIT_FAILURE);
}
bitmap = argv[1];
--
2.41.0
Laszlo Ersek
2023-Jul-27 11:41 UTC
[Libguestfs] [libnbd PATCH 1/7] api: Expose "qemu:" meta-context constants
On 7/26/23 19:29, Eric Blake wrote:> Qemu has a couple of documented meta-context definitions[1]; we might > as well expose constants for these namespaces. > > "qemu:dirty-bitmap:NAME" is a set of namespaces for any arbitrary > dirty bitmap name; we can't define constants for every bitspace name, > but it is possible to do NBD_OPT_LIST_META_CONTEXT on > "qemu:dirty-bitmap:" to see which dirty bitmaps are available. When a > dirty bitmap is negotiated, only one bit is defined (an extent is > dirty or not). The presence of '-' and ':' in the context name beyond > the namespace requires a new helper function. > > "qemu:allocation-depth" returns an integer rather than a bitmap (0 for > unmapped, 1 for current image, 2 and beyond for number of files in the > backing chain before the data was supplied), so we can't really define > any constants for interpreting its values. > > [1] https://gitlab.com/qemu-project/qemu/-/blob/master/docs/interop/nbd.txt > > For libnbd.h, the generated diff is: > > | --- include/libnbd.h.bak 2023-07-26 11:01:45.401328604 -0500 > | +++ include/libnbd.h 2023-07-26 11:59:38.289021067 -0500 > | @@ -1083,6 +1083,16 @@ > | #define LIBNBD_STATE_HOLE 1 > | #define LIBNBD_STATE_ZERO 2 > | > | +/* "qemu" namespace */ > | +#define LIBNBD_NAMESPACE_QEMU "qemu:" > | + > | +/* "qemu" namespace contexts */ > | +#define LIBNBD_CONTEXT_QEMU_DIRTY_BITMAP "qemu:dirty-bitmap:" > | +#define LIBNBD_CONTEXT_QEMU_ALLOCATION_DEPTH "qemu:allocation-depth" > | + > | +/* "qemu:dirty-bitmap:" context related constants */ > | +#define LIBNBD_STATE_DIRTY 1 > | + > | #ifdef __cplusplus > | } > | #endif > > Signed-off-by: Eric Blake <eblake at redhat.com> > --- > generator/utils.mli | 1 + > generator/API.ml | 9 ++++++--- > generator/C.ml | 22 +++++++++++++++------- > generator/GoLang.ml | 3 ++- > generator/OCaml.ml | 4 ++-- > generator/Python.ml | 3 ++- > generator/utils.ml | 5 +++++ > interop/dirty-bitmap.c | 6 ++++-- > 8 files changed, 37 insertions(+), 16 deletions(-)I'm not really convinced this is helpful. - Libnbd is not supposed to be tied to any particular NBD server AIUI, so open-coding QEMU-specific constants in the library header (for which we promise stability, in C) seems unneeded and risky. (I do see the last hunk for the interop/ directory -- I'm unsure what that directory is for.) - In the other direction, we don't implement the whole story (for "qemu:allocation-depth"). In theory, we could introduce symbolic constants for 0 and 1, such as LIBNBD_STATE_UNMAPPED and LIBNBD_STATE_CURRENT (and client code could use ">LIBNBD_STATE_CURRENT", i.e., ">1", equivalently to ">=2"). - I *think* this patch is not needed for patch#2. Anyway, I don't want to block this patch just because I'm not convinced enough to review it in detail :) So if Rich likes it, I'm certainly game. Acked-by: Laszlo Ersek <lersek at redhat.com> Thanks Laszlo> > diff --git a/generator/utils.mli b/generator/utils.mli > index 773e705f..c4d47a34 100644 > --- a/generator/utils.mli > +++ b/generator/utils.mli > @@ -46,6 +46,7 @@ val > val cspan : string -> string -> int > val quote : string -> string > val spaces : int -> string > +val macro_name : string -> string > val files_equal : string -> string -> bool > > val generate_header : > diff --git a/generator/API.ml b/generator/API.ml > index 02c1260d..72c81657 100644 > --- a/generator/API.ml > +++ b/generator/API.ml > @@ -3919,9 +3919,12 @@ let constants > > let metadata_namespaces = [ > "base", [ "allocation", [ > - "STATE_HOLE", 1 lsl 0; > - "STATE_ZERO", 1 lsl 1; > - ] ]; > + "STATE_HOLE", 1 lsl 0; > + "STATE_ZERO", 1 lsl 1; > + ] ]; > + "qemu", [ "dirty-bitmap:", [ "STATE_DIRTY", 1 lsl 0; ]; > + "allocation-depth", []; > + ]; > ] > > let pod_of_link = function > diff --git a/generator/C.ml b/generator/C.ml > index f772117c..a2670f70 100644 > --- a/generator/C.ml > +++ b/generator/C.ml > @@ -373,13 +373,18 @@ let > pr "#define LIBNBD_HAVE_NBD_%s 1\n" name_upper; > pr "\n" > > -let print_ns_ctxt ns ns_upper ctxt consts > - let ctxt_upper = String.uppercase_ascii ctxt in > +let print_ns_ctxt ns ns_upper ctxt > + let ctxt_macro = macro_name ctxt in > + let ctxt_upper = String.uppercase_ascii ctxt_macro in > pr "#define LIBNBD_CONTEXT_%s_%s \"%s:%s\"\n" > - ns_upper ctxt_upper ns ctxt; > - pr "\n"; > - pr "/* \"%s:%s\" context related constants */\n" ns ctxt; > - List.iter (fun (n, i) -> pr "#define LIBNBD_%-30s %d\n" n i) consts > + ns_upper ctxt_upper ns ctxt > + > +let print_ns_ctxt_bits ns ctxt consts > + if consts <> [] then ( > + pr "\n"; > + pr "/* \"%s:%s\" context related constants */\n" ns ctxt; > + List.iter (fun (n, i) -> pr "#define LIBNBD_%-30s %d\n" n i) consts > + ) > > let print_ns ns ctxts > let ns_upper = String.uppercase_ascii ns in > @@ -388,7 +393,10 @@ let > pr "\n"; > pr "/* \"%s\" namespace contexts */\n" ns; > List.iter ( > - fun (ctxt, consts) -> print_ns_ctxt ns ns_upper ctxt consts > + fun (ctxt, _) -> print_ns_ctxt ns ns_upper ctxt > + ) ctxts; > + List.iter ( > + fun (ctxt, consts) -> print_ns_ctxt_bits ns ctxt consts > ) ctxts; > pr "\n" > > diff --git a/generator/GoLang.ml b/generator/GoLang.ml > index 50ed7226..7a7e7f4b 100644 > --- a/generator/GoLang.ml > +++ b/generator/GoLang.ml > @@ -472,7 +472,8 @@ let > pr " namespace_%s = \"%s:\"\n" ns ns; > List.iter ( > fun (ctxt, consts) -> > - pr " context_%s_%s = \"%s:%s\"\n" ns ctxt ns ctxt; > + let ctxt_macro = macro_name ctxt in > + pr " context_%s_%s = \"%s:%s\"\n" ns ctxt_macro ns ctxt; > List.iter (fun (n, v) -> > pr " %s uint32 = %d\n" n v > ) consts > diff --git a/generator/OCaml.ml b/generator/OCaml.ml > index edb81f25..efc1af22 100644 > --- a/generator/OCaml.ml > +++ b/generator/OCaml.ml > @@ -183,7 +183,7 @@ type > pr "val namespace_%s : string\n" ns; > List.iter ( > fun (ctxt, consts) -> > - pr "val context_%s_%s : string\n" ns ctxt; > + pr "val context_%s_%s : string\n" ns (macro_name ctxt); > List.iter (fun (n, _) -> > pr "val %s : int32\n" (String.lowercase_ascii n) > ) consts > @@ -315,7 +315,7 @@ let > pr "let namespace_%s = \"%s:\"\n" ns ns; > List.iter ( > fun (ctxt, consts) -> > - pr "let context_%s_%s = \"%s:%s\"\n" ns ctxt ns ctxt; > + pr "let context_%s_%s = \"%s:%s\"\n" ns (macro_name ctxt) ns ctxt; > List.iter (fun (n, i) -> > pr "let %s = %d_l\n" (String.lowercase_ascii n) i > ) consts > diff --git a/generator/Python.ml b/generator/Python.ml > index c81878de..beaeaa4c 100644 > --- a/generator/Python.ml > +++ b/generator/Python.ml > @@ -745,7 +745,8 @@ let > pr "NAMESPACE_%s = \"%s:\"\n" ns_upper ns; > List.iter ( > fun (ctxt, consts) -> > - let ctxt_upper = String.uppercase_ascii ctxt in > + let ctxt_macro = macro_name ctxt in > + let ctxt_upper = String.uppercase_ascii ctxt_macro in > pr "%s = \"%s:%s\"\n" > (sprintf "CONTEXT_%s_%s" ns_upper ctxt_upper) ns ctxt; > List.iter (fun (n, i) -> pr "%s = %d\n" n i) consts > diff --git a/generator/utils.ml b/generator/utils.ml > index e0c67d20..61cce877 100644 > --- a/generator/utils.ml > +++ b/generator/utils.ml > @@ -151,6 +151,11 @@ let > > let spaces n = String.make n ' ' > > +(* Convert s to macro name by changing '-' to '_' and eliding ':'. *) > +let macro_name s > + let underscore = Str.global_replace (Str.regexp_string "-") "_" s in > + Str.global_replace (Str.regexp ":") "" underscore > + > (* Save the current output channel and replace it with a temporary buffer while > * running ?code?. Return the buffer. > *) > diff --git a/interop/dirty-bitmap.c b/interop/dirty-bitmap.c > index 05f6e9db..16faaed9 100644 > --- a/interop/dirty-bitmap.c > +++ b/interop/dirty-bitmap.c > @@ -127,8 +127,10 @@ main (int argc, char *argv[]) > nbd_extent_callback extent_callback = { .callback = cb, .user_data = &data }; > char c; > > - if (argc < 3) { > - fprintf (stderr, "%s bitmap qemu-nbd [args ...]\n", argv[0]); > + if (argc < 3 || strncmp (argv[1], LIBNBD_CONTEXT_QEMU_DIRTY_BITMAP, > + strlen (LIBNBD_CONTEXT_QEMU_DIRTY_BITMAP)) != 0) { > + fprintf (stderr, "%s qemu:dirty-bitmap:name qemu-nbd [args ...]\n", > + argv[0]); > exit (EXIT_FAILURE); > } > bitmap = argv[1]; > > > _______________________________________________ > Libguestfs mailing list > Libguestfs at redhat.com > https://listman.redhat.com/mailman/listinfo/libguestfs