8:25 a.m.
This picks up python and ocaml. Some of our languages are lacking a
number of bindings (for example, lua and perl lack .extents, so I
didn't have anything to copy from), and I felt less comfortable with
golang and rust. But for python and ocaml, I was able to test a
working implementation.
Eric Blake (2):
python: Implement .list_exports and friends
ocaml: Implement .list_exports and friends
plugins/python/nbdkit-python-plugin.pod | 25 ++++
tests/Makefile.am | 3 +
plugins/ocaml/NBDKit.mli | 9 ++
plugins/ocaml/NBDKit.ml | 17 +++
plugins/ocaml/example.ml | 33 +++--
plugins/ocaml/ocaml.c | 89 ++++++++++++
plugins/python/python.c | 185 ++++++++++++++++++++----
tests/test-python-export-list.sh | 69 +++++++++
tests/python-export-list.py | 54 +++++++
9 files changed, 444 insertions(+), 40 deletions(-)
create mode 100755 tests/test-python-export-list.sh
create mode 100644 tests/python-export-list.py
--
2.28.0
8:25 a.m.
New subject: [nbdkit PATCH 1/2] python: Implement .list_exports and friends
Fairly straightforward. .list_exports uses the same idiom as .extents
for returning an iterable of tuples, with additional support for a
bare name rather than a name/desc tuple. .default_export and
.export_description are rather easy clients of nbdkit_strdup_intern.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
plugins/python/nbdkit-python-plugin.pod | 25 ++++
tests/Makefile.am | 3 +
plugins/python/python.c | 185 ++++++++++++++++++++----
tests/test-python-export-list.sh | 69 +++++++++
tests/python-export-list.py | 54 +++++++
5 files changed, 306 insertions(+), 30 deletions(-)
create mode 100755 tests/test-python-export-list.sh
create mode 100644 tests/python-export-list.py
diff --git a/plugins/python/nbdkit-python-plugin.pod
b/plugins/python/nbdkit-python-plugin.pod
index ddae677e..19b20088 100644
--- a/plugins/python/nbdkit-python-plugin.pod
+++ b/plugins/python/nbdkit-python-plugin.pod
@@ -178,6 +178,23 @@ See L</Threads> below.
There are no arguments or return value.
+=item C<list_exports>
+
+(Optional)
+
+ def list_exports(readonly, is_tls):
+ # return an iterable object (eg. list) of
+ # (name, description) tuples or bare names:
+ return [ (name1, desc1), name2, (name3, desc3), ... ]
+
+=item C<default_export>
+
+(Optional)
+
+ def default_export(readonly, is_tls):
+ # return a string
+ return "name"
+
=item C<open>
(Required)
@@ -199,6 +216,14 @@ After C<close> returns, the reference count of the handle is
decremented in the C part, which usually means that the handle and its
contents will be garbage collected.
+=item C<export_description>
+
+(Optional)
+
+ def export_description(h):
+ # return a string
+ return "description"
+
=item C<get_size>
(Required)
diff --git a/tests/Makefile.am b/tests/Makefile.am
index cbbc750a..7c0e5c5f 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -1075,16 +1075,19 @@ TESTS += \
test-python.sh \
test-python-exception.sh \
test-python-export-name.sh \
+ test-python-export-list.sh \
test-python-thread-model.sh \
test-shebang-python.sh \
$(NULL)
EXTRA_DIST += \
python-exception.py \
python-export-name.py \
+ python-export-list.py \
python-thread-model.py \
shebang.py \
test-python-exception.sh \
test-python-export-name.sh \
+ test-python-export-list.sh \
test-python-plugin.py \
test-python-thread-model.sh \
test-python.sh \
diff --git a/plugins/python/python.c b/plugins/python/python.c
index 27c5ede2..498ffebc 100644
--- a/plugins/python/python.c
+++ b/plugins/python/python.c
@@ -536,6 +536,99 @@ py_get_ready (void)
return 0;
}
+static int
+py_list_exports (int readonly, int is_tls, struct nbdkit_exports *exports)
+{
+ ACQUIRE_PYTHON_GIL_FOR_CURRENT_SCOPE;
+ PyObject *fn;
+ PyObject *r;
+ PyObject *iter, *t;
+
+ if (!callback_defined ("list_exports", &fn))
+ /* Do the same as the core server */
+ return nbdkit_add_default_export (exports);
+
+ PyErr_Clear ();
+
+ r = PyObject_CallFunction (fn, "ii", readonly, is_tls);
+ Py_DECREF (fn);
+ if (check_python_failure ("list_exports") == -1)
+ return -1;
+
+ iter = PyObject_GetIter (r);
+ if (iter == NULL) {
+ nbdkit_error ("list_exports method did not return "
+ "something which is iterable");
+ Py_DECREF (r);
+ return -1;
+ }
+
+ while ((t = PyIter_Next (iter)) != NULL) {
+ PyObject *py_name, *py_desc;
+ CLEANUP_FREE char *name = NULL;
+ CLEANUP_FREE char *desc = NULL;
+
+ name = python_to_string (t);
+ if (!name) {
+ if (!PyTuple_Check (t) || PyTuple_Size (t) != 2) {
+ nbdkit_error ("list_exports method did not return an iterable of "
+ "2-tuples");
+ Py_DECREF (iter);
+ Py_DECREF (r);
+ return -1;
+ }
+ py_name = PyTuple_GetItem (t, 0);
+ py_desc = PyTuple_GetItem (t, 1);
+ name = python_to_string (py_name);
+ desc = python_to_string (py_desc);
+ if (name == NULL || desc == NULL) {
+ nbdkit_error ("list_exports method did not return an iterable of "
+ "string 2-tuples");
+ Py_DECREF (iter);
+ Py_DECREF (r);
+ return -1;
+ }
+ }
+ if (nbdkit_add_export (exports, name, desc) == -1) {
+ Py_DECREF (iter);
+ Py_DECREF (r);
+ return -1;
+ }
+ }
+
+ Py_DECREF (iter);
+ Py_DECREF (r);
+ return 0;
+}
+
+static const char *
+py_default_export (int readonly, int is_tls)
+{
+ ACQUIRE_PYTHON_GIL_FOR_CURRENT_SCOPE;
+ PyObject *fn;
+ PyObject *r;
+ CLEANUP_FREE char *name = NULL;
+
+ if (!callback_defined ("default_export", &fn))
+ return "";
+
+ PyErr_Clear ();
+
+ r = PyObject_CallFunction (fn, "ii", readonly, is_tls);
+ Py_DECREF (fn);
+ if (check_python_failure ("default_export") == -1)
+ return NULL;
+
+ name = python_to_string (r);
+ Py_DECREF (r);
+ if (!name) {
+ nbdkit_error ("default_export method did not return a string");
+ return NULL;
+ }
+
+ return nbdkit_strdup_intern (name);
+}
+
struct handle {
int can_zero;
PyObject *py_h;
@@ -595,6 +688,35 @@ py_close (void *handle)
free (h);
}
+static const char *
+py_export_description (void *handle)
+{
+ ACQUIRE_PYTHON_GIL_FOR_CURRENT_SCOPE;
+ struct handle *h = handle;
+ PyObject *fn;
+ PyObject *r;
+ CLEANUP_FREE char *desc = NULL;
+
+ if (!callback_defined ("export_description", &fn))
+ return NULL;
+
+ PyErr_Clear ();
+
+ r = PyObject_CallFunctionObjArgs (fn, h->py_h, NULL);
+ Py_DECREF (fn);
+ if (check_python_failure ("export_description") == -1)
+ return NULL;
+
+ desc = python_to_string (r);
+ Py_DECREF (r);
+ if (!desc) {
+ nbdkit_error ("export_description method did not return a string");
+ return NULL;
+ }
+
+ return nbdkit_strdup_intern (desc);
+}
+
static int64_t
py_get_size (void *handle)
{
@@ -1120,42 +1242,45 @@ py_extents (void *handle, uint32_t count, uint64_t offset,
#define THREAD_MODEL NBDKIT_THREAD_MODEL_PARALLEL
static struct nbdkit_plugin plugin = {
- .name = "python",
- .version = PACKAGE_VERSION,
+ .name = "python",
+ .version = PACKAGE_VERSION,
- .load = py_load,
- .unload = py_unload,
- .dump_plugin = py_dump_plugin,
+ .load = py_load,
+ .unload = py_unload,
+ .dump_plugin = py_dump_plugin,
- .config = py_config,
- .config_complete = py_config_complete,
- .config_help = py_config_help,
+ .config = py_config,
+ .config_complete = py_config_complete,
+ .config_help = py_config_help,
- .thread_model = py_thread_model,
- .get_ready = py_get_ready,
+ .thread_model = py_thread_model,
+ .get_ready = py_get_ready,
+ .list_exports = py_list_exports,
+ .default_export = py_default_export,
- .open = py_open,
- .close = py_close,
+ .open = py_open,
+ .close = py_close,
- .get_size = py_get_size,
- .is_rotational = py_is_rotational,
- .can_multi_conn = py_can_multi_conn,
- .can_write = py_can_write,
- .can_flush = py_can_flush,
- .can_trim = py_can_trim,
- .can_zero = py_can_zero,
- .can_fast_zero = py_can_fast_zero,
- .can_fua = py_can_fua,
- .can_cache = py_can_cache,
- .can_extents = py_can_extents,
+ .export_description = py_export_description,
+ .get_size = py_get_size,
+ .is_rotational = py_is_rotational,
+ .can_multi_conn = py_can_multi_conn,
+ .can_write = py_can_write,
+ .can_flush = py_can_flush,
+ .can_trim = py_can_trim,
+ .can_zero = py_can_zero,
+ .can_fast_zero = py_can_fast_zero,
+ .can_fua = py_can_fua,
+ .can_cache = py_can_cache,
+ .can_extents = py_can_extents,
- .pread = py_pread,
- .pwrite = py_pwrite,
- .flush = py_flush,
- .trim = py_trim,
- .zero = py_zero,
- .cache = py_cache,
- .extents = py_extents,
+ .pread = py_pread,
+ .pwrite = py_pwrite,
+ .flush = py_flush,
+ .trim = py_trim,
+ .zero = py_zero,
+ .cache = py_cache,
+ .extents = py_extents,
};
NBDKIT_REGISTER_PLUGIN (plugin)
diff --git a/tests/test-python-export-list.sh b/tests/test-python-export-list.sh
new file mode 100755
index 00000000..7c86a1e0
--- /dev/null
+++ b/tests/test-python-export-list.sh
@@ -0,0 +1,69 @@
+#!/usr/bin/env bash
+# nbdkit
+# Copyright (C) 2018-2020 Red Hat Inc.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+# * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#
+# * Redistributions in binary form must reproduce the above copyright
+# notice, this list of conditions and the following disclaimer in the
+# documentation and/or other materials provided with the distribution.
+#
+# * Neither the name of Red Hat nor the names of its contributors may be
+# used to endorse or promote products derived from this software without
+# specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS IS'' AND
+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
+# PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR
+# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+# USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+# OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+# SUCH DAMAGE.
+
+source ./functions.sh
+set -e
+set -x
+
+if test ! -d "$SRCDIR"; then
+ echo "$0: could not locate python-export-list.py"
+ exit 1
+fi
+
+# Python has proven very difficult to valgrind, therefore it is disabled.
+if [ "$NBDKIT_VALGRIND" = "1" ]; then
+ echo "$0: skipping Python test under valgrind."
+ exit 77
+fi
+
+requires nbdinfo --version
+requires nbdsh -c 'print (h.set_full_info)'
+requires jq --version
+
+pid=test-python-export-list.pid
+sock=`mktemp -u`
+out=test-python-export-list.out
+files="$pid $sock $out"
+rm -f $files
+cleanup_fn rm -f $files
+
+start_nbdkit -P $pid -U $sock python $SRCDIR/python-export-list.py
+
+nbdinfo --list --json nbd+unix://\?socket=$sock > $out
+cat $out
+diff -u <(jq -c '[.exports[] | [."export-name", .description]]'
$out) \
+ <(printf %s\\n '[["hello","world"],["name
only",null]]')
+
+nbdinfo --json nbd+unix://\?socket=$sock > $out
+cat $out
+diff -u <(jq -c '[.exports[] | [."export-name", .description]]'
$out) \
+ <(printf %s\\n '[["hello","=hello="]]')
diff --git a/tests/python-export-list.py b/tests/python-export-list.py
new file mode 100644
index 00000000..b0ed8f8c
--- /dev/null
+++ b/tests/python-export-list.py
@@ -0,0 +1,54 @@
+# nbdkit
+# Copyright (C) 2018-2020 Red Hat Inc.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+# * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#
+# * Redistributions in binary form must reproduce the above copyright
+# notice, this list of conditions and the following disclaimer in the
+# documentation and/or other materials provided with the distribution.
+#
+# * Neither the name of Red Hat nor the names of its contributors may be
+# used to endorse or promote products derived from this software without
+# specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS IS'' AND
+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
+# PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR
+# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+# USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+# OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+# SUCH DAMAGE.
+
+# Create an nbdkit sh plugin which reflects the export name back to
+# the caller in the virtual device data and size.
+
+import builtins
+import nbdkit
+
+def list_exports(readonly, is_tls):
+ return [ ("hello", "world"), "name only" ]
+
+def default_export(readonly, is_tls):
+ return "hello"
+
+def open(readonly):
+ return nbdkit.export_name()
+
+def export_description(h):
+ return "=%s=" % h
+
+def get_size(h):
+ return 0
+
+def pread(h, count, offset):
+ pass
--
2.28.0
9:35 a.m.
New subject: [nbdkit PATCH 1/2] python: Implement .list_exports and friends
I didn't check this one in detail, but it has a test case so ACK.
We'll find out soon enough if someone tries to write a Python
plugin that uses this feature.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html
8:25 a.m.
New subject: [nbdkit PATCH 2/2] ocaml: Implement .list_exports and friends
Fairly straightforward. I'd love for type export to be a bit more
flexible to make description optional, but could not figure out how to
decode that from the C side of things, so for now this just requires
the caller to supply a description for all exports during
.list_exports.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
plugins/ocaml/NBDKit.mli | 9 ++++
plugins/ocaml/NBDKit.ml | 17 ++++++++
plugins/ocaml/example.ml | 33 ++++++++++-----
plugins/ocaml/ocaml.c | 89 ++++++++++++++++++++++++++++++++++++++++
4 files changed, 138 insertions(+), 10 deletions(-)
diff --git a/plugins/ocaml/NBDKit.mli b/plugins/ocaml/NBDKit.mli
index 3ebbf18f..0d7e325b 100644
--- a/plugins/ocaml/NBDKit.mli
+++ b/plugins/ocaml/NBDKit.mli
@@ -51,6 +51,12 @@ type extent = {
}
(** The type of the extent list returned by [.extents]. *)
+type export = {
+ name : string;
+ description : string;
+}
+(** The type of the export list returned by [.list_exports]. *)
+
type thread_model =
| THREAD_MODEL_SERIALIZE_CONNECTIONS
| THREAD_MODEL_SERIALIZE_ALL_REQUESTS
@@ -78,10 +84,13 @@ type 'a plugin = {
after_fork : (unit -> unit) option;
preconnect : (bool -> unit) option;
+ list_exports : (bool -> bool -> export list) option;
+ default_export : (bool -> bool -> string) option;
open_connection : (bool -> 'a) option; (* required *)
close : ('a -> unit) option;
get_size : ('a -> int64) option; (* required *)
+ export_description : ('a -> string) option;
can_cache : ('a -> cache_flag) option;
can_extents : ('a -> bool) option;
diff --git a/plugins/ocaml/NBDKit.ml b/plugins/ocaml/NBDKit.ml
index 9ce3bf3e..1d014934 100644
--- a/plugins/ocaml/NBDKit.ml
+++ b/plugins/ocaml/NBDKit.ml
@@ -53,6 +53,11 @@ type extent = {
is_zero : bool;
}
+type export = {
+ name : string;
+ description : string;
+}
+
type 'a plugin = {
name : string;
longname : string;
@@ -73,10 +78,13 @@ type 'a plugin = {
after_fork : (unit -> unit) option;
preconnect : (bool -> unit) option;
+ list_exports : (bool -> bool -> export list) option;
+ default_export : (bool -> bool -> string) option;
open_connection : (bool -> 'a) option;
close : ('a -> unit) option;
get_size : ('a -> int64) option;
+ export_description : ('a -> string) option;
can_cache : ('a -> cache_flag) option;
can_extents : ('a -> bool) option;
@@ -118,10 +126,13 @@ let default_callbacks = {
after_fork = None;
preconnect = None;
+ list_exports = None;
+ default_export = None;
open_connection = None;
close = None;
get_size = None;
+ export_description = None;
can_cache = None;
can_extents = None;
@@ -162,10 +173,13 @@ external set_get_ready : (unit -> unit) -> unit =
"ocaml_nbdkit_set_get_ready"
external set_after_fork : (unit -> unit) -> unit =
"ocaml_nbdkit_set_after_fork"
external set_preconnect : (bool -> unit) -> unit =
"ocaml_nbdkit_set_preconnect"
+external set_list_exports : (bool -> bool -> export list) -> unit =
"ocaml_nbdkit_set_list_exports"
+external set_default_export : (bool -> bool -> string) -> unit =
"ocaml_nbdkit_set_default_export"
external set_open : (bool -> 'a) -> unit = "ocaml_nbdkit_set_open"
external set_close : ('a -> unit) -> unit = "ocaml_nbdkit_set_close"
external set_get_size : ('a -> int64) -> unit =
"ocaml_nbdkit_set_get_size"
+external set_export_description : ('a -> string) -> unit =
"ocaml_nbdkit_set_export_description"
external set_can_cache : ('a -> cache_flag) -> unit =
"ocaml_nbdkit_set_can_cache"
external set_can_extents : ('a -> bool) -> unit =
"ocaml_nbdkit_set_can_extents"
@@ -225,10 +239,13 @@ let register_plugin plugin =
may set_after_fork plugin.after_fork;
may set_preconnect plugin.preconnect;
+ may set_list_exports plugin.list_exports;
+ may set_default_export plugin.default_export;
may set_open plugin.open_connection;
may set_close plugin.close;
may set_get_size plugin.get_size;
+ may set_export_description plugin.export_description;
may set_can_cache plugin.can_cache;
may set_can_extents plugin.can_extents;
diff --git a/plugins/ocaml/example.ml b/plugins/ocaml/example.ml
index 448de8c4..5dc7b374 100644
--- a/plugins/ocaml/example.ml
+++ b/plugins/ocaml/example.ml
@@ -41,6 +41,13 @@ let ocamlexample_config key value =
| _ ->
failwith (Printf.sprintf "unknown parameter: %s" key)
+let ocamlexample_list_exports ro tls : NBDKit.export list =
+ [ { name = "name1"; description = "desc1" };
+ { name = "name2"; description = "desc2" } ]
+
+let ocamlexample_default_export ro tls =
+ "name1"
+
(* Any type (even unit) can be used as a per-connection handle.
* This is just an example. The same value that you return from
* your [open_connection] function is passed back as the first
@@ -58,6 +65,9 @@ let ocamlexample_open readonly =
incr id;
{ h_id = !id }
+let ocamlexample_export_description h =
+ "some description"
+
let ocamlexample_get_size h =
Int64.of_int (Bytes.length !disk)
@@ -80,20 +90,23 @@ let plugin = {
(* name, open_connection, get_size and pread are required,
* everything else is optional.
*)
- NBDKit.name = "ocamlexample";
- version = "1.0";
+ NBDKit.name = "ocamlexample";
+ version = "1.0";
- load = Some ocamlexample_load;
- unload = Some ocamlexample_unload;
+ load = Some ocamlexample_load;
+ unload = Some ocamlexample_unload;
- config = Some ocamlexample_config;
+ config = Some ocamlexample_config;
- open_connection = Some ocamlexample_open;
- get_size = Some ocamlexample_get_size;
- pread = Some ocamlexample_pread;
- pwrite = Some ocamlexample_pwrite;
+ list_exports = Some ocamlexample_list_exports;
+ default_export = Some ocamlexample_default_export;
+ open_connection = Some ocamlexample_open;
+ export_description = Some ocamlexample_export_description;
+ get_size = Some ocamlexample_get_size;
+ pread = Some ocamlexample_pread;
+ pwrite = Some ocamlexample_pwrite;
- thread_model = Some ocamlexample_thread_model;
+ thread_model = Some ocamlexample_thread_model;
}
let () = NBDKit.register_plugin plugin
diff --git a/plugins/ocaml/ocaml.c b/plugins/ocaml/ocaml.c
index d8b61108..a34f67ca 100644
--- a/plugins/ocaml/ocaml.c
+++ b/plugins/ocaml/ocaml.c
@@ -123,10 +123,13 @@ static value get_ready_fn;
static value after_fork_fn;
static value preconnect_fn;
+static value list_exports_fn;
+static value default_export_fn;
static value open_fn;
static value close_fn;
static value get_size_fn;
+static value export_description_fn;
static value can_cache_fn;
static value can_extents_fn;
@@ -311,6 +314,64 @@ preconnect_wrapper (int readonly)
CAMLreturnT (int, 0);
}
+static int
+list_exports_wrapper (int readonly, int is_tls, struct nbdkit_exports *exports)
+{
+ CAMLparam0 ();
+ CAMLlocal2 (rv, v);
+
+ caml_leave_blocking_section ();
+
+ rv = caml_callback2_exn (list_exports_fn, Val_bool (readonly),
+ Val_bool (is_tls));
+ if (Is_exception_result (rv)) {
+ nbdkit_error ("%s", caml_format_exception (Extract_exception (rv)));
+ caml_enter_blocking_section ();
+ CAMLreturnT (int, -1);
+ }
+
+ /* Convert exports list into calls to nbdkit_add_export. */
+ while (rv != Val_int (0)) {
+ const char *name, *desc;
+
+ v = Field (rv, 0); /* export struct */
+ name = String_val (Field (v, 0));
+ desc = String_val (Field (v, 1));
+ if (nbdkit_add_export (exports, name, desc) == -1) {
+ caml_enter_blocking_section ();
+ CAMLreturnT (int, -1);
+ }
+
+ rv = Field (rv, 1);
+ }
+
+ caml_enter_blocking_section ();
+ CAMLreturnT (int, 0);
+}
+
+static const char *
+default_export_wrapper (int readonly, int is_tls)
+{
+ const char *name;
+ CAMLparam0 ();
+ CAMLlocal1 (rv);
+
+ caml_leave_blocking_section ();
+
+ rv = caml_callback2_exn (default_export_fn, Val_bool (readonly),
+ Val_bool (is_tls));
+ if (Is_exception_result (rv)) {
+ nbdkit_error ("%s", caml_format_exception (Extract_exception (rv)));
+ caml_enter_blocking_section ();
+ CAMLreturnT (const char *, NULL);
+ }
+
+ name = nbdkit_strdup_intern (String_val (rv));
+
+ caml_enter_blocking_section ();
+ CAMLreturnT (const char *, name);
+}
+
static void *
open_wrapper (int readonly)
{
@@ -358,6 +419,28 @@ close_wrapper (void *h)
CAMLreturn0;
}
+static const char *
+export_description_wrapper (void *h)
+{
+ const char *desc;
+ CAMLparam0 ();
+ CAMLlocal1 (rv);
+
+ caml_leave_blocking_section ();
+
+ rv = caml_callback_exn (export_description_fn, *(value *) h);
+ if (Is_exception_result (rv)) {
+ nbdkit_error ("%s", caml_format_exception (Extract_exception (rv)));
+ caml_enter_blocking_section ();
+ CAMLreturnT (const char *, NULL);
+ }
+
+ desc = nbdkit_strdup_intern (String_val (rv));
+
+ caml_enter_blocking_section ();
+ CAMLreturnT (const char *, desc);
+}
+
static int64_t
get_size_wrapper (void *h)
{
@@ -856,10 +939,13 @@ SET(get_ready)
SET(after_fork)
SET(preconnect)
+SET(list_exports)
+SET(default_export)
SET(open)
SET(close)
SET(get_size)
+SET(export_description)
SET(can_write)
SET(can_flush)
@@ -900,10 +986,13 @@ remove_roots (void)
REMOVE (after_fork);
REMOVE (preconnect);
+ REMOVE (list_exports);
+ REMOVE (default_export);
REMOVE (open);
REMOVE (close);
REMOVE (get_size);
+ REMOVE (export_description);
REMOVE (can_cache);
REMOVE (can_extents);
--
2.28.0
8:41 a.m.
New subject: [nbdkit PATCH 2/2] ocaml: Implement .list_exports and friends
On 9/1/20 8:25 AM, Eric Blake wrote:
Fairly straightforward. I'd love for type export to be a bit
more
flexible to make description optional, but could not figure out how to
decode that from the C side of things, so for now this just requires
the caller to supply a description for all exports during
.list_exports.
Maybe I did figure it out after all, although I'm still not sure this is
the best interface. Applying this on top of the original patch lets me
use 'string option' instead of 'string' as the second member of the
export record.
https://caml.inria.fr/pub/docs/manual-ocaml/intfc.html#s%3Ac-ocaml-dataty...
didn't directly answer my question, but my understanding is that since
'string option' is the same as:
type 'a t = a' option =
| None (* Is_block is false, value is Val_int(0) *)
| Some of 'a (* Is_block is true, value is block with tag 0 *)
then checking Is_block tells me whether I have None or Some string, at
which point another Field() deref gets me to the string contained in
that block.
diff --git i/plugins/ocaml/NBDKit.mli w/plugins/ocaml/NBDKit.mli
index 0d7e325b..aaec519c 100644
--- i/plugins/ocaml/NBDKit.mli
+++ w/plugins/ocaml/NBDKit.mli
@@ -53,7 +53,7 @@ type extent = {
type export = {
name : string;
- description : string;
+ description : string option;
}
(** The type of the export list returned by [.list_exports]. *)
diff --git i/plugins/ocaml/NBDKit.ml w/plugins/ocaml/NBDKit.ml
index 1d014934..1823fc71 100644
--- i/plugins/ocaml/NBDKit.ml
+++ w/plugins/ocaml/NBDKit.ml
@@ -55,7 +55,7 @@ type extent = {
type export = {
name : string;
- description : string;
+ description : string option;
}
type 'a plugin = {
diff --git i/plugins/ocaml/example.ml w/plugins/ocaml/example.ml
index 5dc7b374..de493bf2 100644
--- i/plugins/ocaml/example.ml
+++ w/plugins/ocaml/example.ml
@@ -42,8 +42,8 @@ let ocamlexample_config key value =
failwith (Printf.sprintf "unknown parameter: %s" key)
let ocamlexample_list_exports ro tls : NBDKit.export list =
- [ { name = "name1"; description = "desc1" };
- { name = "name2"; description = "desc2" } ]
+ [ { name = "name1"; description = Some "desc1" };
+ { name = "name2"; description = None } ]
let ocamlexample_default_export ro tls =
"name1"
diff --git i/plugins/ocaml/ocaml.c w/plugins/ocaml/ocaml.c
index a34f67ca..ea499454 100644
--- i/plugins/ocaml/ocaml.c
+++ w/plugins/ocaml/ocaml.c
@@ -332,11 +332,12 @@ list_exports_wrapper (int readonly, int is_tls,
struct nbdkit_exports *exports)
/* Convert exports list into calls to nbdkit_add_export. */
while (rv != Val_int (0)) {
- const char *name, *desc;
+ const char *name, *desc = NULL;
v = Field (rv, 0); /* export struct */
name = String_val (Field (v, 0));
- desc = String_val (Field (v, 1));
+ if (Is_block (Field (v, 1)))
+ desc = String_val (Field (Field (v, 1), 0));
if (nbdkit_add_export (exports, name, desc) == -1) {
caml_enter_blocking_section ();
CAMLreturnT (int, -1);
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
9:13 a.m.
New subject: [nbdkit PATCH 2/2] ocaml: Implement .list_exports and friends
On Tue, Sep 01, 2020 at 08:41:40AM -0500, Eric Blake wrote:
On 9/1/20 8:25 AM, Eric Blake wrote:
>Fairly straightforward. I'd love for type export to be a bit more
>flexible to make description optional, but could not figure out how to
>decode that from the C side of things, so for now this just requires
>the caller to supply a description for all exports during
>.list_exports.
>
Maybe I did figure it out after all, although I'm still not sure
this is the best interface. Applying this on top of the original
patch lets me use 'string option' instead of 'string' as the second
member of the export record.
https://caml.inria.fr/pub/docs/manual-ocaml/intfc.html#s%3Ac-ocaml-dataty...
didn't directly answer my question, but my understanding is that
since 'string option' is the same as:
type 'a t = a' option =
| None (* Is_block is false, value is Val_int(0) *)
| Some of 'a (* Is_block is true, value is block with tag 0 *)
then checking Is_block tells me whether I have None or Some string,
at which point another Field() deref gets me to the string contained
in that block.
All ‘value’ things in OCaml are just 64 bit integers. The only
difference is whether the bottom bit is 0, in which case it's a
pointer to an OCaml block, or 1 in which case it's an integer-like
thing shifted left by one bit. Is_block() simply checks the bottom
bit.
Val_int(0) turns C int 0 into an OCaml int. Which is done by shifting
it left one place and setting the bottom bit. IOW it returns C 1.
Also None is represented as OCaml 0 (so C 1):
https://rwmj.wordpress.com/2009/08/04/ocaml-internals/
OCaml blocks are lists of values allocated on the OCaml heap:
https://rwmj.wordpress.com/2009/08/05/ocaml-internals-part-2-strings-and-...
and those values may be ints or pointers.
Therefore the easiest way to tell if something is None from C is
to compare it with Val_int(0), ie:
value v = /* something which has type ‘string option’ */;
if (v == Val_int(0)) /* It's None */
printf ("None\n");
else {
/* It's Some string, set sv to the string value */
value sv = Field (v, 0);
printf ("Some %s\n", String_val (sv));
}
The weird casting macros have the form: To_from()
Also that String_val is only valid for a short period of time,
basically until the next OCaml allocation in the current thread.
+++ w/plugins/ocaml/ocaml.c
@@ -332,11 +332,12 @@ list_exports_wrapper (int readonly, int
is_tls, struct nbdkit_exports *exports)
/* Convert exports list into calls to nbdkit_add_export. */
while (rv != Val_int (0)) {
- const char *name, *desc;
+ const char *name, *desc = NULL;
v = Field (rv, 0); /* export struct */
name = String_val (Field (v, 0));
- desc = String_val (Field (v, 1));
+ if (Is_block (Field (v, 1)))
+ desc = String_val (Field (Field (v, 1), 0));
if (nbdkit_add_export (exports, name, desc) == -1) {
caml_enter_blocking_section ();
CAMLreturnT (int, -1);
Seems OK. I'm guessing this crashes?
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html
9:27 a.m.
New subject: [nbdkit PATCH 2/2] ocaml: Implement .list_exports and friends
On 9/1/20 9:13 AM, Richard W.M. Jones wrote:
On Tue, Sep 01, 2020 at 08:41:40AM -0500, Eric Blake wrote:
> On 9/1/20 8:25 AM, Eric Blake wrote:
>> Fairly straightforward. I'd love for type export to be a bit more
>> flexible to make description optional, but could not figure out how to
>> decode that from the C side of things, so for now this just requires
>> the caller to supply a description for all exports during
>> .list_exports.
>>
>
OCaml blocks are lists of values allocated on the OCaml heap:
https://rwmj.wordpress.com/2009/08/05/ocaml-internals-part-2-strings-and-...
and those values may be ints or pointers.
Therefore the easiest way to tell if something is None from C is
to compare it with Val_int(0), ie:
value v = /* something which has type ‘string option’ */;
if (v == Val_int(0)) /* It's None */
printf ("None\n");
else {
/* It's Some string, set sv to the string value */
value sv = Field (v, 0);
printf ("Some %s\n", String_val (sv));
}
The weird casting macros have the form: To_from()
Also that String_val is only valid for a short period of time,
basically until the next OCaml allocation in the current thread.
That's helpful, as well.
> +++ w/plugins/ocaml/ocaml.c
> @@ -332,11 +332,12 @@ list_exports_wrapper (int readonly, int
> is_tls, struct nbdkit_exports *exports)
>
> /* Convert exports list into calls to nbdkit_add_export. */
> while (rv != Val_int (0)) {
> - const char *name, *desc;
> + const char *name, *desc = NULL;
>
> v = Field (rv, 0); /* export struct */
> name = String_val (Field (v, 0));
> - desc = String_val (Field (v, 1));
> + if (Is_block (Field (v, 1)))
> + desc = String_val (Field (Field (v, 1), 0));
> if (nbdkit_add_export (exports, name, desc) == -1) {
> caml_enter_blocking_section ();
> CAMLreturnT (int, -1);
Seems OK. I'm guessing this crashes?
Nope, it worked. So compared to your explanation above, my test for
Some was Is_block, rather than Int_val(0). And I repeated Field(v,1)
for getting to the 'Some "string"' rather than an intermediate variable,
but matched your code in the Field(x, 0) for accessing the string
portion of it.
I guess my remaining questions are whether there is a better approach than:
func : export list =
[ { "name1"; None }; { "name2"; Some "desc" } ]
In python, I was able to use:
return [ "name1", ( "name2", "desc" ) ]
where the alternation was on a string vs. a 2-tuple of strings, rather
than all list members being a record but where the record had an
optional member. But I'm not sure how to represent that in the ocaml
interface. Maybe I'm thinking of something like:
type export =
| Name of string
| NameDesc of string * string
but I'm not quite sure how to write a list that initializes that. On
the other hand, I think I understand the C binding for such a type:
either val is Val_int(0) (C 1) for the Name branch (Field(v, 0) is a
string), or val is Val_int(1) (C 2) for the NameDesc branch (Field(v, 0)
is name, Field(v, 1) is desc).
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
9:45 a.m.
New subject: [nbdkit PATCH 2/2] ocaml: Implement .list_exports and friends
On Tue, Sep 01, 2020 at 09:27:39AM -0500, Eric Blake wrote:
I guess my remaining questions are whether there is a better approach
than:
func : export list =
[ { "name1"; None }; { "name2"; Some "desc" } ]
In python, I was able to use:
return [ "name1", ( "name2", "desc" ) ]
where the alternation was on a string vs. a 2-tuple of strings,
rather than all list members being a record but where the record had
an optional member. But I'm not sure how to represent that in the
ocaml interface. Maybe I'm thinking of something like:
type export =
| Name of string
| NameDesc of string * string
but I'm not quite sure how to write a list that initializes that.
On the other hand, I think I understand the C binding for such a
type: either val is Val_int(0) (C 1) for the Name branch (Field(v,
0) is a string), or val is Val_int(1) (C 2) for the NameDesc branch
(Field(v, 0) is name, Field(v, 1) is desc).
Using optional fields in a struct (your original implementation) is
fine and perfectly natural.
If you really wanted to use the Name/NameDesc version then you'd write
that in OCaml as:
[ Name "name1"; NameDesc ("name2", "desc") ]
and on the C side each element of the list would be a block with tags
0 (Name) or 1 (NameDesc), and 1 or 2 fields respectively.
The way constructors are stored is complicated and optimized for fast
access rather than ease of unpacking for C programmers. If you really
want the details it's at the bottom of:
https://rwmj.wordpress.com/2009/08/05/ocaml-internals-part-2-strings-and-...
or in the official manual here:
https://caml.inria.fr/pub/docs/manual-ocaml/intfc.html#ss:c-concrete-data...
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
9:38 a.m.
New subject: [nbdkit PATCH 2/2] ocaml: Implement .list_exports and friends
On Tue, Sep 01, 2020 at 03:13:28PM +0100, Richard W.M. Jones wrote:
Val_int(0) turns C int 0 into an OCaml int. Which is done by
shifting
it left one place and setting the bottom bit. IOW it returns C 1.
That was confusing.
I mean Val_int(0) returns the C representation of the OCaml integer 0
(or integer-like thing: None is also represented as the OCaml integer 0).
This is also the reason why OCaml ints are 63 bits, and you must use
the OCaml int64 type (which is boxed) to store a true 64 bit int.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines. Supports shell scripting,
bindings from many languages. http://libguestfs.org
1573
days inactive
1573
days old
8 comments
2 participants
participants (2)
-
Eric Blake -
Richard W.M. Jones