thr3ads.net - Libguestfs - [Libguestfs] [libnbd PATCH 0/7] Preliminary GoLang cleanups [Jul 2023]

If this information is useful, please help other people find it:
Share via:

Eric Blake

2023-Jul-26 17:29 UTC

[Libguestfs] [libnbd PATCH 0/7] Preliminary GoLang cleanups

In Laszlo's review of my 64-bit extensions, he pointed out some
questionable code in the GoLang.ml generated code, including some
inconsistencies on whitespace before '('.  Fixing that in turn
inspired me to learn about gofmt, so I attempted to address the issues
it points out.  (One perk of being a modern language: you can enforce
everyone to share the same style, rather than having different camps
each defend their years-long but slightly distinct styles...)

We may want to optionally run gofmt as part of the build process (I
know Tage proposed doing similar with rustfmt), and/or write a further
patch to have the const() blocks in GoLang.ml do a two-pass iteration
over lists (first to learn the maximum column width, second to do the
output).

Eric Blake (7):
  api: Expose "qemu:" meta-context constants
  golang: Export meta-context constants
  golang: Simplify RBool return
  Revert "generator/Go.ml: Simplify copy_uint32_array"
  golang: Use 'gofmt' style recommendations on manual files
  golang: Improve whitespace style in generated bindings
  golang: Enforce coding style during 'make check'

 generator/utils.mli                           |   1 +
 generator/API.ml                              |   9 +-
 generator/C.ml                                |  22 +-
 generator/GoLang.ml                           | 246 +++++++++---------
 generator/OCaml.ml                            |   4 +-
 generator/Python.ml                           |   3 +-
 generator/utils.ml                            |   5 +
 interop/dirty-bitmap.c                        |   6 +-
 golang/Makefile.am                            |   2 +-
 golang/codestyle-tests.sh                     |  45 ++++
 golang/configure/test.go                      |  14 +-
 golang/examples/aio_copy/aio_copy.go          |   7 +-
 golang/examples/simple_copy/simple_copy.go    |   3 +-
 golang/libnbd_220_opt_list_test.go            |   4 +-
 golang/libnbd_230_opt_info_test.go            |  24 +-
 golang/libnbd_240_opt_list_meta_test.go       |  44 ++--
 .../libnbd_245_opt_list_meta_queries_test.go  |  14 +-
 golang/libnbd_250_opt_set_meta_test.go        |  20 +-
 .../libnbd_255_opt_set_meta_queries_test.go   |  14 +-
 golang/libnbd_405_pread_structured_test.go    |  10 +-
 golang/libnbd_460_block_status_test.go        |   4 +-
 golang/libnbd_590_aio_copy_test.go            |  21 +-
 golang/libnbd_620_stats_test.go               |  12 +-
 23 files changed, 307 insertions(+), 227 deletions(-)
 create mode 100755 golang/codestyle-tests.sh


base-commit: 5c2fc3cc7e14146d000b65b191e70d9a0585a395
-- 
2.41.0

Eric Blake

2023-Jul-26 17:29 UTC

head link

[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

Eric Blake

2023-Jul-26 17:29 UTC

head link

[Libguestfs] [libnbd PATCH 2/7] golang: Export meta-context constants

Go insists that a module's members are not public unless they start
with a capital letter.  Our attempt at defining constants
'namespace_base' and 'context_base_allocation' were thus only
visible
to our in-package tests, and not for external clients.

Although this patch does not completely address gofmt complaints,
adding some strategic grouping comments inside the const() block has
an additional benefit of reducing the amount of whitespace that gofmt
wants to inject into unrelated lines.

The change to generated golang/bindigs.go is:

| --- golang/bindings.go.bak	2023-07-26 12:06:47.618324437 -0500
| +++ golang/bindings.go	2023-07-26 12:06:55.521385295 -0500
| @@ -111,14 +111,18 @@
|      READ_DATA uint32 = 1
|      READ_HOLE uint32 = 2
|      READ_ERROR uint32 = 3
| -    namespace_base = "base:"
| -    context_base_allocation = "base:allocation"
| +    /* Meta-context namespace "base" */
| +    NAMESPACE_BASE = "base:"
| +    CONTEXT_BASE_ALLOCATION = "base:allocation"
| +    /* Defined bits in "base:allocation" */
|      STATE_HOLE uint32 = 1
|      STATE_ZERO uint32 = 2
| -    namespace_qemu = "qemu:"
| -    context_qemu_dirty_bitmap = "qemu:dirty-bitmap:"
| +    /* Meta-context namespace "qemu" */
| +    NAMESPACE_QEMU = "qemu:"
| +    CONTEXT_QEMU_DIRTY_BITMAP = "qemu:dirty-bitmap:"
| +    /* Defined bits in "qemu:dirty-bitmap:" */
|      STATE_DIRTY uint32 = 1
| -    context_qemu_allocation_depth = "qemu:allocation-depth"
| +    CONTEXT_QEMU_ALLOCATION_DEPTH = "qemu:allocation-depth"
|  )
|
|  /* SetDebug: set or clear the debug flag */

Signed-off-by: Eric Blake <eblake at redhat.com>

---

RFC: Do we want MixedCase rather than ALL_CAPS names?  If so, this
patch needs to be rethought a bit, as it would impact API for existing
public constants as well as the ones changed here.  But we don't
promise API stability for Go.
---
 generator/GoLang.ml                           | 10 +++++---
 golang/libnbd_230_opt_info_test.go            | 24 +++++++++----------
 golang/libnbd_240_opt_list_meta_test.go       | 14 +++++------
 .../libnbd_245_opt_list_meta_queries_test.go  |  4 ++--
 golang/libnbd_250_opt_set_meta_test.go        | 20 ++++++++--------
 .../libnbd_255_opt_set_meta_queries_test.go   | 12 +++++-----
 golang/libnbd_460_block_status_test.go        |  4 ++--
 7 files changed, 46 insertions(+), 42 deletions(-)

diff --git a/generator/GoLang.ml b/generator/GoLang.ml
index 7a7e7f4b..82d73ed6 100644
--- a/generator/GoLang.ml
+++ b/generator/GoLang.ml
@@ -469,11 +469,15 @@ let
   ) constants;
   List.iter (
     fun (ns, ctxts) ->
-      pr "    namespace_%s = \"%s:\"\n" ns ns;
+      let ns_upper = String.uppercase_ascii ns in
+      pr "    /* Meta-context namespace \"%s\" */\n" ns;
+      pr "    NAMESPACE_%s = \"%s:\"\n" ns_upper ns;
       List.iter (
         fun (ctxt, consts) ->
-          let ctxt_macro = macro_name ctxt in
-          pr "    context_%s_%s = \"%s:%s\"\n" ns
ctxt_macro ns ctxt;
+          let ctxt_macro = String.uppercase_ascii (macro_name ctxt) in
+          pr "    CONTEXT_%s_%s = \"%s:%s\"\n" ns_upper
ctxt_macro ns ctxt;
+          if consts <> [] then
+            pr "    /* Defined bits in \"%s:%s\" */\n" ns
ctxt;
           List.iter (fun (n, v) ->
               pr "    %s uint32 = %d\n" n v
           ) consts
diff --git a/golang/libnbd_230_opt_info_test.go
b/golang/libnbd_230_opt_info_test.go
index cd00be9e..cc7c2d91 100644
--- a/golang/libnbd_230_opt_info_test.go
+++ b/golang/libnbd_230_opt_info_test.go
@@ -45,7 +45,7 @@ func Test230OptInfo(t *testing.T) {
 		t.Fatalf("could not connect: %s", err)
 	}

-	err = h.AddMetaContext(context_base_allocation)
+	err = h.AddMetaContext(CONTEXT_BASE_ALLOCATION)
 	if err != nil {
 		t.Fatalf("could not add meta context: %s", err)
 	}
@@ -59,7 +59,7 @@ func Test230OptInfo(t *testing.T) {
 	if err == nil {
 		t.Fatalf("expected error")
 	}
-	_, err = h.CanMetaContext(context_base_allocation)
+	_, err = h.CanMetaContext(CONTEXT_BASE_ALLOCATION)
 	if err == nil {
 		t.Fatalf("expected error")
 	}
@@ -83,7 +83,7 @@ func Test230OptInfo(t *testing.T) {
 	if !ro {
 		t.Fatalf("unexpected readonly")
 	}
-	meta, err := h.CanMetaContext(context_base_allocation)
+	meta, err := h.CanMetaContext(CONTEXT_BASE_ALLOCATION)
 	if err != nil {
 		t.Fatalf("can_meta failed unexpectedly: %s", err)
 	}
@@ -104,7 +104,7 @@ func Test230OptInfo(t *testing.T) {
 	if err == nil {
 		t.Fatalf("expected error")
 	}
-	_, err = h.CanMetaContext(context_base_allocation)
+	_, err = h.CanMetaContext(CONTEXT_BASE_ALLOCATION)
 	if err == nil {
 		t.Fatalf("expected error")
 	}
@@ -151,7 +151,7 @@ func Test230OptInfo(t *testing.T) {
 	if ro {
 		t.Fatalf("unexpected readonly")
 	}
-	_, err = h.CanMetaContext(context_base_allocation)
+	_, err = h.CanMetaContext(CONTEXT_BASE_ALLOCATION)
 	if err == nil {
 		t.Fatalf("expected error")
 	}
@@ -177,7 +177,7 @@ func Test230OptInfo(t *testing.T) {
 	if err == nil {
 		t.Fatalf("expected error")
 	}
-	_, err = h.CanMetaContext(context_base_allocation)
+	_, err = h.CanMetaContext(CONTEXT_BASE_ALLOCATION)
 	if err == nil {
 		t.Fatalf("expected error")
 	}
@@ -205,7 +205,7 @@ func Test230OptInfo(t *testing.T) {
 	if !ro {
 		t.Fatalf("unexpected readonly")
 	}
-	meta, err = h.CanMetaContext(context_base_allocation)
+	meta, err = h.CanMetaContext(CONTEXT_BASE_ALLOCATION)
 	if err != nil {
 		t.Fatalf("can_meta failed unexpectedly: %s", err)
 	}
@@ -236,7 +236,7 @@ func Test230OptInfo(t *testing.T) {
 	if size != 4 {
 		t.Fatalf("unexpected size")
 	}
-	meta, err = h.CanMetaContext(context_base_allocation)
+	meta, err = h.CanMetaContext(CONTEXT_BASE_ALLOCATION)
 	if err != nil {
 		t.Fatalf("can_meta failed unexpectedly: %s", err)
 	}
@@ -270,7 +270,7 @@ func Test230OptInfo(t *testing.T) {
 		t.Fatalf("could not add meta context: %s", err)
 	}

-	_, err = h.CanMetaContext(context_base_allocation)
+	_, err = h.CanMetaContext(CONTEXT_BASE_ALLOCATION)
 	if err == nil {
 		t.Fatalf("expected error")
 	}
@@ -278,7 +278,7 @@ func Test230OptInfo(t *testing.T) {
 	if err != nil {
 		t.Fatalf("opt_info failed unexpectedly: %s", err)
 	}
-	meta, err = h.CanMetaContext(context_base_allocation)
+	meta, err = h.CanMetaContext(CONTEXT_BASE_ALLOCATION)
 	if err != nil {
 		t.Fatalf("can_meta failed unexpectedly: %s", err)
 	}
@@ -290,7 +290,7 @@ func Test230OptInfo(t *testing.T) {
 		t.Fatalf("set request meta context failed unexpectedly: %s", err)
 	}
 	/* Adding to the request list now won't matter */
-	err = h.AddMetaContext(context_base_allocation)
+	err = h.AddMetaContext(CONTEXT_BASE_ALLOCATION)
 	if err != nil {
 		t.Fatalf("could not add meta context: %s", err)
 	}
@@ -298,7 +298,7 @@ func Test230OptInfo(t *testing.T) {
 	if err != nil {
 		t.Fatalf("opt_go failed unexpectedly: %s", err)
 	}
-	meta, err = h.CanMetaContext(context_base_allocation)
+	meta, err = h.CanMetaContext(CONTEXT_BASE_ALLOCATION)
 	if err != nil {
 		t.Fatalf("can_meta failed unexpectedly: %s", err)
 	}
diff --git a/golang/libnbd_240_opt_list_meta_test.go
b/golang/libnbd_240_opt_list_meta_test.go
index 0235fe3f..011b5704 100644
--- a/golang/libnbd_240_opt_list_meta_test.go
+++ b/golang/libnbd_240_opt_list_meta_test.go
@@ -31,7 +31,7 @@ func listmetaf(user_data int, name string) int {
 		panic("expected user_data == 42")
 	}
 	list_count++
-	if (name == context_base_allocation) {
+	if (name == CONTEXT_BASE_ALLOCATION) {
 		list_seen = true
 	}
 	return 0
@@ -92,7 +92,7 @@ func Test240OptListMeta(t *testing.T) {
 	/* Third pass: specific query should have one match. */
 	list_count = 0
 	list_seen = false
-	err = h.AddMetaContext(context_base_allocation)
+	err = h.AddMetaContext(CONTEXT_BASE_ALLOCATION)
 	if err != nil {
 		t.Fatalf("could not request add_meta_context: %s", err)
 	}
@@ -107,7 +107,7 @@ func Test240OptListMeta(t *testing.T) {
 	if err != nil {
 		t.Fatalf("could not request get_meta_context: %s", err)
 	}
-	if *tmp != context_base_allocation {
+	if *tmp != CONTEXT_BASE_ALLOCATION {
 		t.Fatalf("wrong result of get_meta_context: %s", *tmp)
 	}
 	r, err = h.OptListMetaContext(func(name string) int {
@@ -129,7 +129,7 @@ func Test240OptListMeta(t *testing.T) {
 	if err == nil {
 		t.Fatalf("expected error")
 	}
-	_, err = h.CanMetaContext(context_base_allocation)
+	_, err = h.CanMetaContext(CONTEXT_BASE_ALLOCATION)
 	if err == nil {
 		t.Fatalf("expected error")
 	}
@@ -144,7 +144,7 @@ func Test240OptListMeta(t *testing.T) {
         if size != 1048576 {
 		t.Fatalf("get_size gave wrong size")
         }
-	meta, err := h.CanMetaContext(context_base_allocation)
+	meta, err := h.CanMetaContext(CONTEXT_BASE_ALLOCATION)
 	if err != nil {
 		t.Fatalf("can_meta_context failed unexpectedly: %s", err)
 	}
@@ -175,7 +175,7 @@ func Test240OptListMeta(t *testing.T) {
         if size != 1048576 {
 		t.Fatalf("get_size gave wrong size")
         }
-	meta, err = h.CanMetaContext(context_base_allocation)
+	meta, err = h.CanMetaContext(CONTEXT_BASE_ALLOCATION)
 	if err != nil {
 		t.Fatalf("can_meta_context failed unexpectedly: %s", err)
 	}
@@ -187,7 +187,7 @@ func Test240OptListMeta(t *testing.T) {
 	/* Final pass: "base:" query should get at least
"base:allocation" */
 	list_count = 0
 	list_seen = false
-	err = h.AddMetaContext("base:")
+	err = h.AddMetaContext(NAMESPACE_BASE)
 	if err != nil {
 		t.Fatalf("could not request add_meta_context: %s", err)
 	}
diff --git a/golang/libnbd_245_opt_list_meta_queries_test.go
b/golang/libnbd_245_opt_list_meta_queries_test.go
index fc1ab60c..3ae2d854 100644
--- a/golang/libnbd_245_opt_list_meta_queries_test.go
+++ b/golang/libnbd_245_opt_list_meta_queries_test.go
@@ -28,7 +28,7 @@ func listmetaqf(user_data int, name string) int {
 		panic("expected user_data == 42")
 	}
 	listq_count++
-	if (name == context_base_allocation) {
+	if (name == CONTEXT_BASE_ALLOCATION) {
 		listq_seen = true
 	}
 	return 0
@@ -97,7 +97,7 @@ func(name string) int {
 	listq_count = 0
 	listq_seen = false
 	r, err = h.OptListMetaContextQueries([]string{
-		"x-nosuch:", context_base_allocation },
+		"x-nosuch:", CONTEXT_BASE_ALLOCATION },
 		func(name string) int {
 	        return listmetaqf(42, name)
 		})
diff --git a/golang/libnbd_250_opt_set_meta_test.go
b/golang/libnbd_250_opt_set_meta_test.go
index 6d27fa22..4263ede5 100644
--- a/golang/libnbd_250_opt_set_meta_test.go
+++ b/golang/libnbd_250_opt_set_meta_test.go
@@ -28,7 +28,7 @@ func setmetaf(user_data int, name string) int {
 		panic("expected user_data == 42")
 	}
 	set_count++
-	if (name == context_base_allocation) {
+	if (name == CONTEXT_BASE_ALLOCATION) {
 		set_seen = true
 	}
 	return 0
@@ -67,18 +67,18 @@ func Test250OptSetMeta(t *testing.T) {
 	if sr {
 		t.Fatalf("unexpected structured replies state")
 	}
-	meta, err := h.CanMetaContext(context_base_allocation)
+	meta, err := h.CanMetaContext(CONTEXT_BASE_ALLOCATION)
 	if err != nil {
 		t.Fatalf("could not check can meta context: %s", err)
 	}
 	if meta {
 		t.Fatalf("unexpected can meta context state")
 	}
-	err = h.AddMetaContext(context_base_allocation)
+	err = h.AddMetaContext(CONTEXT_BASE_ALLOCATION)
 	if err != nil {
 		t.Fatalf("could not request add_meta_context: %s", err)
 	}
-	_, err = h.CanMetaContext(context_base_allocation)
+	_, err = h.CanMetaContext(CONTEXT_BASE_ALLOCATION)
 	if err == nil {
 		t.Fatalf("expected error")
 	}
@@ -109,7 +109,7 @@ func Test250OptSetMeta(t *testing.T) {
 	if !sr {
 		t.Fatalf("unexpected structured replies state")
 	}
-	_, err = h.CanMetaContext(context_base_allocation)
+	_, err = h.CanMetaContext(CONTEXT_BASE_ALLOCATION)
 	if err == nil {
 		t.Fatalf("expected error")
 	}
@@ -151,7 +151,7 @@ func Test250OptSetMeta(t *testing.T) {
 	if r != set_count || r != 0 || set_seen {
 		t.Fatalf("unexpected set_count after opt_set_meta_context")
 	}
-	meta, err = h.CanMetaContext(context_base_allocation)
+	meta, err = h.CanMetaContext(CONTEXT_BASE_ALLOCATION)
 	if err != nil {
 		t.Fatalf("could not check can meta context: %s", err)
 	}
@@ -166,7 +166,7 @@ func Test250OptSetMeta(t *testing.T) {
 	if err != nil {
 		t.Fatalf("could not request add_meta_context: %s", err)
 	}
-	err = h.AddMetaContext(context_base_allocation)
+	err = h.AddMetaContext(CONTEXT_BASE_ALLOCATION)
 	if err != nil {
 		t.Fatalf("could not request add_meta_context: %s", err)
 	}
@@ -183,7 +183,7 @@ func Test250OptSetMeta(t *testing.T) {
 	if r != 1 || r != set_count || !set_seen {
 		t.Fatalf("unexpected set_count after opt_set_meta_context")
 	}
-	meta, err = h.CanMetaContext(context_base_allocation)
+	meta, err = h.CanMetaContext(CONTEXT_BASE_ALLOCATION)
 	if err != nil {
 		t.Fatalf("could not check can meta context: %s", err)
 	}
@@ -204,7 +204,7 @@ func Test250OptSetMeta(t *testing.T) {
 	if err != nil {
 		t.Fatalf("could not request opt_go: %s", err)
 	}
-	meta, err = h.CanMetaContext(context_base_allocation)
+	meta, err = h.CanMetaContext(CONTEXT_BASE_ALLOCATION)
 	if err != nil {
 		t.Fatalf("could not check can meta context: %s", err)
 	}
@@ -224,7 +224,7 @@ func Test250OptSetMeta(t *testing.T) {
 	if set_count != 0 || set_seen {
 		t.Fatalf("unexpected set_count after opt_set_meta_context")
 	}
-	meta, err = h.CanMetaContext(context_base_allocation)
+	meta, err = h.CanMetaContext(CONTEXT_BASE_ALLOCATION)
 	if err != nil {
 		t.Fatalf("could not check can meta context: %s", err)
 	}
diff --git a/golang/libnbd_255_opt_set_meta_queries_test.go
b/golang/libnbd_255_opt_set_meta_queries_test.go
index cca25ae7..232560bc 100644
--- a/golang/libnbd_255_opt_set_meta_queries_test.go
+++ b/golang/libnbd_255_opt_set_meta_queries_test.go
@@ -28,7 +28,7 @@ func setmetaqf(user_data int, name string) int {
 		panic("expected user_data == 42")
 	}
 	setq_count++
-	if (name == context_base_allocation) {
+	if (name == CONTEXT_BASE_ALLOCATION) {
 		setq_seen = true
 	}
 	return 0
@@ -74,7 +74,7 @@ func(name string) int {
 	 */
 	setq_count = 0
 	setq_seen = false
-	err = h.AddMetaContext(context_base_allocation)
+	err = h.AddMetaContext(CONTEXT_BASE_ALLOCATION)
 	if err != nil {
 		t.Fatalf("could not request add_meta_context: %s", err)
 	}
@@ -87,7 +87,7 @@ func(name string) int {
 	if r != setq_count || r != 0 || setq_seen {
 		t.Fatalf("unexpected set_count after opt_set_meta_context_queries")
 	}
-	meta, err := h.CanMetaContext(context_base_allocation)
+	meta, err := h.CanMetaContext(CONTEXT_BASE_ALLOCATION)
 	if err != nil {
 		t.Fatalf("could not check can meta context: %s", err)
 	}
@@ -99,7 +99,7 @@ func(name string) int {
 	setq_count = 0
 	setq_seen = false
 	r, err = h.OptSetMetaContextQueries([]string{
-		"x-nosuch:context", context_base_allocation},
+		"x-nosuch:context", CONTEXT_BASE_ALLOCATION},
 		func(name string) int {
 	        return setmetaqf(42, name)
 		})
@@ -109,7 +109,7 @@ func(name string) int {
 	if r != 1 || r != setq_count || !setq_seen {
 		t.Fatalf("unexpected set_count after opt_set_meta_context_queries")
 	}
-	meta, err = h.CanMetaContext(context_base_allocation)
+	meta, err = h.CanMetaContext(CONTEXT_BASE_ALLOCATION)
 	if err != nil {
 		t.Fatalf("could not check can meta context: %s", err)
 	}
@@ -126,7 +126,7 @@ func(name string) int {
 	if err != nil {
 		t.Fatalf("could not request opt_go: %s", err)
 	}
-	meta, err = h.CanMetaContext(context_base_allocation)
+	meta, err = h.CanMetaContext(CONTEXT_BASE_ALLOCATION)
 	if err != nil {
 		t.Fatalf("could not check can meta context: %s", err)
 	}
diff --git a/golang/libnbd_460_block_status_test.go
b/golang/libnbd_460_block_status_test.go
index dca0d1b8..d2d11e9c 100644
--- a/golang/libnbd_460_block_status_test.go
+++ b/golang/libnbd_460_block_status_test.go
@@ -31,7 +31,7 @@ func mcf(metacontext string, offset uint64, e []uint32, error
*int) int {
 	if *error != 0 {
 		panic("expected *error == 0")
 	}
-	if metacontext == "base:allocation" {
+	if metacontext == CONTEXT_BASE_ALLOCATION {
 		entries = e
 	}
 	return 0
@@ -69,7 +69,7 @@ func Test460BlockStatus(t *testing.T) {
 	}
 	defer h.Close()

-	err = h.AddMetaContext("base:allocation")
+	err = h.AddMetaContext(CONTEXT_BASE_ALLOCATION)
 	if err != nil {
 		t.Fatalf("%s", err)
 	}
-- 
2.41.0

Eric Blake

2023-Jul-26 17:29 UTC

head link

[Libguestfs] [libnbd PATCH 3/7] golang: Simplify RBool return

Among other things, the 'gofmt' tool flagged our use of a single-line
if/else statement for returning an RBool value, recommending we expand
it to multiple lines.  But we don't need that much typing, when we can
just directly compute the boolean result in place.

Signed-off-by: Eric Blake <eblake at redhat.com>
---
 generator/GoLang.ml | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/generator/GoLang.ml b/generator/GoLang.ml
index 82d73ed6..0aa83bdc 100644
--- a/generator/GoLang.ml
+++ b/generator/GoLang.ml
@@ -366,8 +366,7 @@ let
    | RErr ->
       pr "    return nil\n"
    | RBool ->
-      pr "    r := int (ret)\n";
-      pr "    if r != 0 { return true, nil } else { return false, nil
}\n"
+      pr "    return int (ret) != 0, nil\n"
    | RStaticString ->
       pr "    /* ret is statically allocated, do not free it. */\n";
       pr "    r := C.GoString (ret);\n";
-- 
2.41.0

Eric Blake

2023-Jul-26 17:49 UTC

head link

[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];

base-commit: 5c2fc3cc7e14146d000b65b191e70d9a0585a395
-- 
2.41.0

Eric Blake

2023-Jul-26 17:50 UTC

head link

[Libguestfs] [libnbd PATCH 4/7] Revert "generator/Go.ml: Simplify copy_uint32_array"

This reverts commit 6725fa0e129f9a60d7b89707ef8604e0aeeeaf43, although
with a more modern style.

Casting a C array to a Go slice just to benefit from potential
optimizations in Go's copy(), is rather complex to understand,
especially when we are still copying things (the main reason to treat
a C array as a Go slice is when avoiding a copy has a benefit).
Without a benchmark showing measurable difference in runtime speed,
and considering that network transit time probably dominates the time
spent on block status and its callback, it is not worth the
complexity.  Furthermore, an upcoming patch wants to add a similar
helper function for converting between a list of C and Go structs,
where the copy() trick will not work; and having the two helpers look
alike is beneficial.

Suggested-by: Laszlo Ersek <lersek at redhat.com>
CC: Nir Soffer <nsoffer at redhat.com>
Signed-off-by: Eric Blake <eblake at redhat.com>
---
 generator/GoLang.ml | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/generator/GoLang.ml b/generator/GoLang.ml
index 0aa83bdc..77dacadb 100644
--- a/generator/GoLang.ml
+++ b/generator/GoLang.ml
@@ -509,12 +509,14 @@ let

 /* Closures. */

-func copy_uint32_array (entries *C.uint32_t, count C.size_t) []uint32 {
-    ret := make([]uint32, int (count))
-    // See
https://github.com/golang/go/wiki/cgo#turning-c-arrays-into-go-slices
-    // TODO: Use unsafe.Slice() when we require Go 1.17.
-    s := (*[1<<30]uint32)(unsafe.Pointer(entries))[:count:count]
-    copy(ret, s)
+func copy_uint32_array(entries *C.uint32_t, count C.size_t) []uint32 {
+    ret := make([]uint32, int(count))
+    addr := uintptr(unsafe.Pointer(entries))
+    for i := 0; i < int(count); i++ {
+        ptr := (*C.uint32_t)(unsafe.Pointer(addr))
+        ret[i] = uint32(*ptr)
+        addr += unsafe.Sizeof(*ptr)
+    }
     return ret
 }
 ";

base-commit: 5c2fc3cc7e14146d000b65b191e70d9a0585a395
prerequisite-patch-id: 4db98f7b211c0de9a4095b300970e1973cf0716c
prerequisite-patch-id: 0bb320af5109c1c21e5b76d44e6ec1e7e685fd9f
prerequisite-patch-id: 205525d8ea09e77ea13f43d0720153ed5904dbcd
-- 
2.41.0

Libguestfs - Jul 2023 - [libnbd PATCH 0/7] Preliminary GoLang cleanups

[Libguestfs] [libnbd PATCH 0/7] Preliminary GoLang cleanups

[Libguestfs] [libnbd PATCH 1/7] api: Expose "qemu:" meta-context constants

[Libguestfs] [libnbd PATCH 2/7] golang: Export meta-context constants

[Libguestfs] [libnbd PATCH 3/7] golang: Simplify RBool return

[Libguestfs] [libnbd PATCH 1/7] api: Expose "qemu:" meta-context constants

[Libguestfs] [libnbd PATCH 4/7] Revert "generator/Go.ml: Simplify copy_uint32_array"