Richard W.M. Jones
2023-Jan-04 18:14 UTC
[Libguestfs] [PATCH nbdkit] New plugin: blkio [incomplete]
This is an incomplete outline implementation for a libblkio plugin for
nbdkit. At the moment it only supports reading the same ("capacity")
of the device, and not even reading or writing. I have some questions
about the libblkio API before I can complete the plugin (see below).
The idea here is that by connecting libblkio to NBD we can use the
existing set of scripting tools to script access to devices. For
example you could use Python to read or modify a device:
----------------------------------------------------------------------
$ nbdsh -c - <<'EOF'
from subprocess import *
# Run nbdkit-blkio-plugin.
h.connect_systemd_socket_activation ("nbdkit" "blkio",
"virtio-blk-vhost-user",
"path=unix.sock")
print("device size=%d", h.get_size())
# Dump the boot sector.
bootsect = h.pread(512, 0)
p = Popen("hexdump -C", shell=True, stdin=PIPE)
p.stdin.write(bootsect)
EOF
----------------------------------------------------------------------
So my questions and comments about libblkio:
(1) There is no way to know which properties are readable, writable,
and those which need to be set before or after blkio_connect (see
is_preconnect_property in the plugin). It should be possible to
introspect this information. Also might be nice to be able read a
list of all available properties.
(2) It would be nice if libblkio had a way to enable debugging and
call the caller back for debug messages. We could then redirect the
callbacks into the nbdkit debug API (nbdkit_debug()) where they would
eventually end up on stderr or syslog.
However don't send debug messages to stderr, or at least allow that
behaviour to be overridden.
(3) It seems like some drivers require pre-allocated memory regions,
and since some do that means we might as well implement this. It
also seems like some drivers require file-backed pre-allocated
memory regions, and so we might as well implement that too.
However what is not clear: does memfd_create(2) always allocate
sufficiently aligned memory regions, such that we never need to bother
reading the mem-region-alignment property?
I notice that the example:
https://gitlab.com/libblkio/libblkio/-/blob/main/examples/blkio-copy.c
just passes on this and calls blkio_alloc_mem_region(). Is that the
safest and easiest thing to do which will always work?
(4) As a first pass, I only want to bother implementing blocking mode.
It'll be slow, but it doesn't matter for this application. Also
I've
chosen nbdkit's NBDKIT_THREAD_MODEL_SERIALIZE_ALL_REQUESTS so nbdkit
will serialise all requests (again, only for a very simple first pass).
Looking at: https://libblkio.gitlab.io/libblkio/blkio.html#performing-i-o
seems simple enough but:
(4a) What is the queue number? Always 0? Is it affected by num-entries?
(4b) It's unclear how completions work. If I set min=max=1, will it
return after the whole operation has completed? Do I need to
call it again? What about if the request is very large, can it
get split?
Reading the example
https://gitlab.com/libblkio/libblkio/-/blob/main/examples/blkio-copy.c
it appears that requests cannot ever be split?
Rich.
Using libblkio (https://libblkio.gitlab.io/) this implements a plugin
for reading and writing various disk sources mainly used in
virtualization and other high performance cases. These include: NVMe,
vhost-user, vhost-vdpa and VFIO PCI.
Note the current implementation uses the simplest libblkio API mode
(blocking) and so it is not suitable for high performance access.
---
plugins/blkio/nbdkit-blkio-plugin.pod | 117 +++++++++++
configure.ac | 20 ++
plugins/blkio/Makefile.am | 80 ++++++++
plugins/blkio/blkio.c | 270 ++++++++++++++++++++++++++
README.md | 4 +
TODO | 7 +
6 files changed, 498 insertions(+)
diff --git a/plugins/blkio/nbdkit-blkio-plugin.pod
b/plugins/blkio/nbdkit-blkio-plugin.pod
new file mode 100644
index 000000000..c76784687
--- /dev/null
+++ b/plugins/blkio/nbdkit-blkio-plugin.pod
@@ -0,0 +1,117 @@
+=head1 NAME
+
+nbdkit-blkio-plugin - libblkio plugin for NVMe, vhost-user, vDPA, VFIO
+
+=head1 SYNOPSIS
+
+ nbdkit blkio [driver=]DRIVER [path=FILENAME] [num-queues=N] ...
+
+=head1 DESCRIPTION
+
+nbdkit-blkio-plugin is an L<nbdkit(1)> plugin for using
+L<libblkio|https://libblkio.gitlab.io/> to access various disk sources
+used for high performance applications and virtualization. These
+include: NVMe, vhost-user, vDPA and VFIO.
+
+The first parameter after the plugin name should be the L<libblkio
+driver|https://libblkio.gitlab.io/libblkio/blkio.html#drivers>. For
+example:
+
+ nbdkit blkio virtio-blk-vhost-user path=vhost.sock
+
+=head2 Driver: C<nvme-io_uring>
+
+ nbdkit blkio nvme-io_uring path=/dev/ng0n1
+
+Connect to an NVMe device, issuing commands through Linux io_uring
+(requires Linux E<ge> 5.19).
+
+=head2 Driver: C<virtio-blk-vfio-pci>
+
+ nbdkit blkio virtio-blk-vfio-pci path=/sys/bus/pci/devices/0000:00:01.0
+
+Connect to a PCI device which implements virtio-blk using VFIO. The
+path is the path to the device's sysfs directory (see L<lspci(8)>).
+
+=head2 Driver: C<virtio-blk-vhost-user>
+
+ nbdkit blkio virtio-blk-vhost-user path=vhost.sock
+
+Connect to a vhost-user-blk device, such as one exported by
+L<qemu-storage-daemon(1)>. The path is the vhost-user Unix domain
+socket.
+
+=head2 Driver: C<virtio-blk-vhost-vdpa>
+
+ nbdkit blkio virtio-blk-vhost-vdpa path=chardev
+
+Connect to a vDPA device which might be implemented in software
+(eg. VDUSE) or hardware. The path is the vhost-vdpa character device.
+
+=head2 Driver: C<io_uring>
+
+ nbdkit blkio io_uring path=FILENAME
+
+You can use this driver to access local files and block devices
+through the libblkio C<io_uring> driver, but it is usually better and
+easier to use L<nbdkit-file-plugin(1)>.
+
+=head1 PARAMETERS
+
+=over 4
+
+=item [B<driver=>]DRIVER
+
+The name of the libblkio driver to use.
+
+This parameter is required.
+
+C<driver=> is a magic config key and may be omitted in most cases.
+See L<nbdkit(1)/Magic parameters>.
+
+=item PROPERTYB<=>VALUE
+
+Properties such as C<path>, C<num-entries> etc are translated to
+libblkio properties. Consult the L<libblkio
+documentation|https://libblkio.gitlab.io/libblkio/blkio.html> for a
+complete list.
+
+=item B<get=>PROPERTY
+
+Get (print) the value of a property after connecting. The property is
+fetched and printed in nbdkit debug output, so you will need to use
+the I<--verbose> flag. This is useful for debugging.
+
+=back
+
+=head1 FILES
+
+=over 4
+
+=item F<$plugindir/nbdkit-blkio-plugin.so>
+
+The plugin.
+
+Use C<nbdkit --dump-config> to find the location of C<$plugindir>.
+
+=back
+
+=head1 VERSION
+
+C<nbdkit-blkio-plugin> first appeared in nbdkit 1.34.
+
+=head1 SEE ALSO
+
+L<nbdkit-file-plugin(1)>,
+L<lspci(8)>,
+L<qemu-storage-daemon(1)>,
+L<https://libblkio.gitlab.io>,
+L<https://libblkio.gitlab.io/libblkio/blkio.html>.
+
+=head1 AUTHORS
+
+Richard W.M. Jones
+
+=head1 COPYRIGHT
+
+Copyright (C) 2014-2023 Red Hat Inc.
diff --git a/configure.ac b/configure.ac
index e03374b12..6afeac38e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -74,6 +74,7 @@ lang_plugins="\
tcl \
"
non_lang_plugins="\
+ blkio \
cdi \
curl \
data \
@@ -1094,6 +1095,23 @@ AS_IF([test "x$enable_golang" !=
"xno"],[
],[GOLANG=no])
AM_CONDITIONAL([HAVE_GOLANG],[test "x$GOLANG" != "xno"])
+dnl Check for libblkio (only if you want to compile the blkio plugin).
+AC_ARG_WITH([libblkio],
+ [AS_HELP_STRING([--without-libblkio],
+ [disable blkio plugin @<:@default=check@:>@])],
+ [],
+ [with_libblkio=check])
+AS_IF([test "$with_libblkio" != "no"],[
+ PKG_CHECK_MODULES([LIBBLKIO], [blkio],[
+ printf "libblkio version is "; $PKG_CONFIG --modversion blkio
+ AC_SUBST([LIBBLKIO_CFLAGS])
+ AC_SUBST([LIBBLKIO_LIBS])
+ AC_DEFINE([HAVE_LIBBLKIO],[1],[libblkio found at compile time.])
+ ],
+ [AC_MSG_WARN([libblkio not found, blkio plugin will be disabled])])
+])
+AM_CONDITIONAL([HAVE_LIBBLKIO],[test "x$LIBBLKIO_LIBS" !=
"x"])
+
dnl Check for curl (only if you want to compile the curl plugin).
AC_ARG_WITH([curl],
[AS_HELP_STRING([--without-curl],
@@ -1402,6 +1420,7 @@ AC_CONFIG_FILES([Makefile
include/Makefile
include/nbdkit-version.h
plugins/Makefile
+ plugins/blkio/Makefile
plugins/cc/Makefile
plugins/cdi/Makefile
plugins/curl/Makefile
@@ -1533,6 +1552,7 @@ feature "TLS" test
"x$GNUTLS_LIBS" != "x"
echo
echo "Optional plugins:"
echo
+feature "blkio" test "x$HAVE_LIBBLKIO_TRUE" =
"x"
feature "curl" test "x$HAVE_CURL_TRUE" =
"x"
feature "example4" test "x$HAVE_PERL_TRUE" =
"x"
feature "floppy" test "x$HAVE_ICONV_TRUE" =
"x"
diff --git a/plugins/blkio/Makefile.am b/plugins/blkio/Makefile.am
new file mode 100644
index 000000000..dbedcc2eb
--- /dev/null
+++ b/plugins/blkio/Makefile.am
@@ -0,0 +1,80 @@
+# nbdkit
+# Copyright (C) 2014-2023 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.
+
+include $(top_srcdir)/common-rules.mk
+
+EXTRA_DIST = nbdkit-blkio-plugin.pod
+
+if HAVE_LIBBLKIO
+
+plugin_LTLIBRARIES = nbdkit-blkio-plugin.la
+
+nbdkit_blkio_plugin_la_SOURCES = \
+ blkio.c \
+ $(top_srcdir)/include/nbdkit-plugin.h \
+ $(NULL)
+
+nbdkit_blkio_plugin_la_CPPFLAGS = \
+ -I$(top_srcdir)/include \
+ -I$(top_builddir)/include \
+ -I$(top_srcdir)/common/utils \
+ $(NULL)
+nbdkit_blkio_plugin_la_CFLAGS = \
+ $(WARNINGS_CFLAGS) \
+ $(LIBBLKIO_CFLAGS) \
+ $(NULL)
+nbdkit_blkio_plugin_la_LIBADD = \
+ $(top_builddir)/common/utils/libutils.la \
+ $(IMPORT_LIBRARY_ON_WINDOWS) \
+ $(LIBBLKIO_LIBS) \
+ $(NULL)
+nbdkit_blkio_plugin_la_LDFLAGS = \
+ -module -avoid-version -shared $(NO_UNDEFINED_ON_WINDOWS) \
+ $(NULL)
+if USE_LINKER_SCRIPT
+nbdkit_blkio_plugin_la_LDFLAGS += \
+ -Wl,--version-script=$(top_srcdir)/plugins/plugins.syms
+endif
+
+if HAVE_POD
+
+man_MANS = nbdkit-blkio-plugin.1
+CLEANFILES += $(man_MANS)
+
+nbdkit-blkio-plugin.1: nbdkit-blkio-plugin.pod \
+ $(top_builddir)/podwrapper.pl
+ $(PODWRAPPER) --section=1 --man $@ \
+ --html $(top_builddir)/html/$@.html \
+ $<
+
+endif HAVE_POD
+
+endif
diff --git a/plugins/blkio/blkio.c b/plugins/blkio/blkio.c
new file mode 100644
index 000000000..058850f63
--- /dev/null
+++ b/plugins/blkio/blkio.c
@@ -0,0 +1,270 @@
+/* nbdkit
+ * Copyright (C) 2014-2023 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.
+ */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdbool.h>
+#include <errno.h>
+
+#include <blkio.h>
+
+#define NBDKIT_API_VERSION 2
+#include <nbdkit-plugin.h>
+
+#include "vector.h"
+#include "const-string-vector.h"
+
+/* libblkio could do parallel, but we would need to reimplement this
+ * plugin to use the libblkio event model.
+ */
+#define THREAD_MODEL NBDKIT_THREAD_MODEL_SERIALIZE_ALL_REQUESTS
+
+struct property {
+ const char *name;
+ const char *value;
+};
+DEFINE_VECTOR_TYPE(properties, struct property)
+
+static const char *driver = NULL; /* driver name - required */
+static properties props = empty_vector; /* other command line params */
+static const_string_vector gets = empty_vector; /* get= parameters */
+
+static void
+bio_unload (void)
+{
+ properties_reset (&props);
+ const_string_vector_reset (&gets);
+}
+
+/* Called for each key=value passed on the command line. */
+static int
+bio_config (const char *key, const char *value)
+{
+ if (strcmp (key, "driver") == 0) {
+ if (driver != NULL) {
+ nbdkit_error ("'driver' property set more than once");
+ return -1;
+ }
+ driver = value;
+ }
+ else if (strcmp (key, "get") == 0) {
+ if (const_string_vector_append (&gets, value) == -1)
+ return -1;
+ }
+ else if (strcmp (key, "read-only") == 0) {
+ nbdkit_error ("do not set the libblkio \"read-only\"
parameter, "
+ "use the nbdkit -r flag if read-only is required");
+ return -1;
+ }
+ else /* general property */ {
+ struct property prop = { .name = key, .value = value };
+ if (properties_append (&props, prop) == -1)
+ return -1;
+ }
+
+ return 0;
+}
+
+/* Check the user did pass a driver parameter. */
+static int
+bio_config_complete (void)
+{
+ if (driver == NULL) {
+ nbdkit_error ("you must supply the driver=<DRIVER> parameter
"
+ "after the plugin name on the command line");
+ return -1;
+ }
+
+ return 0;
+}
+
+#define bio_config_help \
+ "driver=<DRIVER> (required) Driver name (eg.
\"nvme-io_uring\").\n" \
+ "PROPERTY=VALUE Set arbitrary libblkio property.\n" \
+ "get=PROPERTY Print property name after connection."
+
+/* XXX Should be possible to query this from libblkio. */
+static bool
+is_preconnect_property (const char *name)
+{
+ return strcmp (name, "can-add-queues") == 0 ||
+ strcmp (name, "driver") == 0 ||
+ strcmp (name, "fd") == 0 ||
+ strcmp (name, "path") == 0 ||
+ strcmp (name, "read-only") == 0;
+}
+
+/* Create the per-connection handle. */
+static void *
+bio_open (int readonly)
+{
+ struct blkio *b;
+ size_t i;
+
+ errno = blkio_create (driver, &b);
+ if (errno != 0) {
+ nbdkit_error ("blkio_create: error opening driver: %s: %m",
driver);
+ return NULL;
+ }
+
+ /* Always set the read-only property to a true or false value. */
+ errno = blkio_set_bool (b, "read-only", !!readonly);
+ if (errno != 0) {
+ nbdkit_error ("error setting property: read-only=%s: %m",
+ readonly ? "true" : "false");
+ blkio_destroy (&b);
+ return NULL;
+ }
+
+ /* Set the pre-connect properties. */
+ for (i = 0; i < props.len; ++i) {
+ const struct property *prop = &props.ptr[i];
+
+ if (is_preconnect_property (prop->name)) {
+ errno = blkio_set_str (b, prop->name, prop->value);
+ if (errno != 0) {
+ nbdkit_error ("error setting property: %s=%s: %m",
+ prop->name, prop->value);
+ blkio_destroy (&b);
+ return NULL;
+ }
+ }
+ }
+
+ /* Connect. */
+ errno = blkio_connect (b);
+ if (errno != 0) {
+ nbdkit_error ("blkio_connect: failed to connect to device: %m");
+ blkio_destroy (&b);
+ return NULL;
+ }
+
+ /* Set the post-connect properties. */
+ for (i = 0; i < props.len; ++i) {
+ const struct property *prop = &props.ptr[i];
+
+ if (! is_preconnect_property (prop->name)) {
+ errno = blkio_set_str (b, prop->name, prop->value);
+ if (errno != 0) {
+ nbdkit_error ("error setting property: %s=%s: %m",
+ prop->name, prop->value);
+ blkio_destroy (&b);
+ return NULL;
+ }
+ }
+ }
+
+ /* Start the block device. */
+ errno = blkio_start (b);
+ if (errno != 0) {
+ nbdkit_error ("blkio_start: failed to start device: %m");
+ blkio_destroy (&b);
+ return NULL;
+ }
+
+ /* Print any properties requested on the command line. */
+ for (i = 0; i < gets.len; ++i) {
+ const char *name = gets.ptr[i];
+ char *value = NULL;
+
+ if (blkio_get_str (b, name, &value) == 0)
+ nbdkit_debug ("get %s = %s", name, value);
+ else
+ nbdkit_debug ("could not get property %s: %m", name);
+ free (value);
+ }
+
+ return b;
+}
+
+/* Close the handle. */
+static void
+bio_close (void *handle)
+{
+ struct blkio *b = handle;
+
+ blkio_destroy (&b);
+}
+
+/* Get the device size. */
+static int64_t
+bio_get_size (void *handle)
+{
+ struct blkio *b = handle;
+ uint64_t r;
+
+ errno = blkio_get_uint64 (b, "capacity", &r);
+ if (errno != 0) {
+ nbdkit_error ("error reading device capacity: %m");
+ return -1;
+ }
+
+ return r;
+}
+
+/* Read data from the device. */
+static int
+bio_pread (void *handle, void *buf, uint32_t count, uint64_t offset,
+ uint32_t flags)
+{
+ nbdkit_error ("XXX NOT IMPL XXX");
+ return -1;
+}
+
+/* Write data to the device. */
+static int
+bio_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offset,
+ uint32_t flags)
+{
+ nbdkit_error ("XXX NOT IMPL XXX");
+ return -1;
+}
+
+static struct nbdkit_plugin plugin = {
+ .name = "blkio",
+ .version = PACKAGE_VERSION,
+ .unload = bio_unload,
+ .config = bio_config,
+ .config_complete = bio_config_complete,
+ .config_help = bio_config_help,
+ .magic_config_key = "driver",
+ .open = bio_open,
+ .close = bio_close,
+ .get_size = bio_get_size,
+ .pread = bio_pread,
+ .pwrite = bio_pwrite,
+ .errno_is_preserved = 1,
+};
+
+NBDKIT_REGISTER_PLUGIN(plugin)
diff --git a/README.md b/README.md
index eff94379b..a204ccf39 100644
--- a/README.md
+++ b/README.md
@@ -118,6 +118,10 @@ For the bittorrent plugin:
* [libtorrent-rasterbar](https://www.libtorrent.org)
+For the blkio plugin:
+
+* [libblkio](https://libblkio.gitlab.io/)
+
For the containerized data importer (CDI) plugin:
* podman
diff --git a/TODO b/TODO
index feeb09289..d4c85df9b 100644
--- a/TODO
+++ b/TODO
@@ -155,6 +155,13 @@ nbdkit-cdi-plugin:
* Look at using skopeo instead of podman pull
(https://github.com/containers/skopeo)
+nbdkit-blkio-plugin:
+
+* Use event-driven mode instead of blocking mode. This involves
+ restructuring the plugin so that there is one or more background
+ threads to handle the events, and nbdkit threads issue requests to
+ these threads. (See how it is done in the VDDK plugin.)
+
Suggestions for language plugins
--------------------------------
--
2.37.3
Richard W.M. Jones
2023-Jan-04 18:15 UTC
[Libguestfs] [PATCH nbdkit] New plugin: blkio [incomplete]
On Wed, Jan 04, 2023 at 06:14:34PM +0000, Richard W.M. Jones wrote:> This is an incomplete outline implementation for a libblkio plugin for > nbdkit. At the moment it only supports reading the same ("capacity")s/same/size/ -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com nbdkit - Flexible, fast NBD server with plugins https://gitlab.com/nbdkit/nbdkit
Richard W.M. Jones
2023-Jan-04 19:01 UTC
[Libguestfs] [PATCH nbdkit] New plugin: blkio [incomplete]
(5) "The application is responsible for thread-safety. No thread synchronization is necessary when a queue is only used from a single thread. Proper synchronization is required when sharing a queue between multiple threads." Does this apply across multiple struct blkio handles? ie. Is there now, or could there be in future, some global state which would be corrupted by parallel calls across multiple handles? This matters because we could use NBDKIT_THREAD_MODEL_SERIALIZE_REQUESTS instead of NBDKIT_THREAD_MODEL_SERIALIZE_ALL_REQUESTS (https://libguestfs.org/nbdkit-plugin.3.html#Threads). 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
Richard W.M. Jones
2023-Jan-04 21:43 UTC
[Libguestfs] [PATCH nbdkit] New plugin: blkio [incomplete]
On Wed, Jan 04, 2023 at 06:14:34PM +0000, Richard W.M. Jones wrote:> (3) It seems like some drivers require pre-allocated memory regions, > and since some do that means we might as well implement this. It > also seems like some drivers require file-backed pre-allocated > memory regions, and so we might as well implement that too. > > However what is not clear: does memfd_create(2) always allocate > sufficiently aligned memory regions, such that we never need to bother > reading the mem-region-alignment property? > > I notice that the example: > https://gitlab.com/libblkio/libblkio/-/blob/main/examples/blkio-copy.c > just passes on this and calls blkio_alloc_mem_region(). Is that the > safest and easiest thing to do which will always work?So this seems to be the reverse of what I thought. In nbdkit plugins ideally we'd like to avoid using bounce buffers if possible. Also NBD requests can be up to (IIRC) 32M, although we can hint to the client to limit them to some smaller number. If I call blkio_alloc_mem_region() unconditionally then it seems as if I always need to use this as a bounce buffer. Plus, is 32M too large for a memory region allocated this way? The example allocates 128K. It seems like for drivers which _don't_ require pre-allocated memory regions then we should avoid calling blkio_alloc_mem_region which would avoid bounce buffers. Some guidance about this would be appreciated. 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
Richard W.M. Jones
2023-Jan-04 22:51 UTC
[Libguestfs] [PATCH nbdkit] New plugin: blkio [incomplete]
Here's an updated version which supports read, write, flush,
write-zeroes, discard and FUA:
https://gitlab.com/rwmjones/nbdkit/-/commits/2023-libblkio/
I was able to access a local file using the io_uring driver using
guestfish. You can try it like this:
$ ./nbdkit -fv blkio io_uring path=/var/tmp/fedora-36.img \
--run ' guestfish --format=raw -a $uri -i '
I haven't tested it exhaustively, but it seems to work OK. Obviously
it will be a bit slow.
It doesn't implement pre-allocated buffers, so advice on how and when
to do that would be welcome.
Rich.
PS: Eric: One thing we lack in the NBD space is a standards
conformance test [-suite].
--
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
Richard W.M. Jones
2023-Jan-07 19:44 UTC
[Libguestfs] [PATCH nbdkit] New plugin: blkio [incomplete]
This is upstream in nbdkit now: https://gitlab.com/nbdkit/nbdkit/-/tree/master/plugins/blkio Another question: (6) vhost-user + "read-only" property acts a bit strangely. After opening virtio-blk-vhost-user it throws an EROFS error if you try to "blkio_start" it. However if you set the "read-only" property to true then it's OK to start it, and subsequent read operations work too. I would expect that any device can be started without needing to set this property, and in general I don't understand why vhost-user is r/o since everything I read about it seems to indicate it's a general purpose writable protocol. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
Stefan Hajnoczi
2023-Jan-11 15:52 UTC
[Libguestfs] [PATCH nbdkit] New plugin: blkio [incomplete]
On Wed, Jan 04, 2023 at 06:14:34PM +0000, Richard W.M. Jones wrote:> This is an incomplete outline implementation for a libblkio plugin for > nbdkit. At the moment it only supports reading the same ("capacity") > of the device, and not even reading or writing. I have some questions > about the libblkio API before I can complete the plugin (see below). > > The idea here is that by connecting libblkio to NBD we can use the > existing set of scripting tools to script access to devices. For > example you could use Python to read or modify a device: > > ---------------------------------------------------------------------- > $ nbdsh -c - <<'EOF' > > from subprocess import * > > # Run nbdkit-blkio-plugin. > h.connect_systemd_socket_activation ("nbdkit" "blkio", > "virtio-blk-vhost-user", > "path=unix.sock") > > print("device size=%d", h.get_size()) > > # Dump the boot sector. > bootsect = h.pread(512, 0) > p = Popen("hexdump -C", shell=True, stdin=PIPE) > p.stdin.write(bootsect) > EOF > ---------------------------------------------------------------------- > > So my questions and comments about libblkio:Hi Rich, This is cool! I've answered below:> > (1) There is no way to know which properties are readable, writable, > and those which need to be set before or after blkio_connect (see > is_preconnect_property in the plugin). It should be possible to > introspect this information. Also might be nice to be able read a > list of all available properties.libblkio doesn't support property introspection. The documentation covers when each property can be read/written (e.g. read/write before started and read-only after started). Why is introspection needed?> (2) It would be nice if libblkio had a way to enable debugging and > call the caller back for debug messages. We could then redirect the > callbacks into the nbdkit debug API (nbdkit_debug()) where they would > eventually end up on stderr or syslog. > > However don't send debug messages to stderr, or at least allow that > behaviour to be overridden.libblkio has no debug messages. What stderr output did you get? Errors are returned via blkio_get_error_msg().> (3) It seems like some drivers require pre-allocated memory regions, > and since some do that means we might as well implement this. It > also seems like some drivers require file-backed pre-allocated > memory regions, and so we might as well implement that too. > > However what is not clear: does memfd_create(2) always allocate > sufficiently aligned memory regions, such that we never need to bother > reading the mem-region-alignment property? > > I notice that the example: > https://gitlab.com/libblkio/libblkio/-/blob/main/examples/blkio-copy.c > just passes on this and calls blkio_alloc_mem_region(). Is that the > safest and easiest thing to do which will always work?Yes, blkio_alloc_mem_region() is the safest and easiest way to allocate correctly-aligned memory. Regarding memfd_create(2), I don't believe the memory has any alignment until it's mmapped and then it will be page-aligned. Normally page alignment is sufficient for "buf-alignment" and "mem-region-alignment", but in theory a loop could be used to increase the size of the memfd to achieve any required alignment. That's why blkio_alloc_mem_region() is exists as an easy API that most people will use instead of allocating memory regions on their own.> > (4) As a first pass, I only want to bother implementing blocking mode. > It'll be slow, but it doesn't matter for this application. Also I've > chosen nbdkit's NBDKIT_THREAD_MODEL_SERIALIZE_ALL_REQUESTS so nbdkit > will serialise all requests (again, only for a very simple first pass). > > Looking at: https://libblkio.gitlab.io/libblkio/blkio.html#performing-i-o > seems simple enough but: > > (4a) What is the queue number? Always 0? Is it affected by num-entries?Yes, the first queue is 0. "num-queues" sets the number of queues. "num-entries" is a different property that some drivers have for sizing rings.> (4b) It's unclear how completions work. If I set min=max=1, will it > return after the whole operation has completed? Do I need to > call it again? What about if the request is very large, can it > get split? > > Reading the example > https://gitlab.com/libblkio/libblkio/-/blob/main/examples/blkio-copy.c > it appears that requests cannot ever be split?min=max=1 blocks until exactly 1 completion occurs. BTW I checked the documentation to see if min_completions/max_completions are covered and I'm not sure how to improve the documentation? There is no need to call it again if you got the completion you were waiting for. The caller must honor the constraints given by the "max-segments", "max-segment-len", and "max-transfer" properties. Requests are not split. Stefan -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 488 bytes Desc: not available URL: <http://listman.redhat.com/archives/libguestfs/attachments/20230111/c5a398cb/attachment.sig>