[PATCH nbdkit 1/3] server: Rename global backend pointer to "top".
by Richard W.M. Jones
It's confusing to use the same terminology for a single backend as for
the linked list of backends. In particular it's often not clear if
we're calling the next backend or the whole chain of backends.
---
server/internal.h | 14 ++++++++++--
server/connections.c | 20 ++++++++---------
server/locks.c | 2 +-
server/main.c | 32 ++++++++++++++--------------
server/plugins.c | 2 +-
server/protocol-handshake-newstyle.c | 8 +++----
server/protocol-handshake.c | 28 ++++++++++++------------
server/protocol.c | 18 ++++++++--------
8 files changed, 67 insertions(+), 57 deletions(-)
diff --git a/server/internal.h b/server/internal.h
index 45ac60ad..c3622671 100644
--- a/server/internal.h
+++ b/server/internal.h
@@ -118,8 +118,18 @@ extern char *unixsocket;
extern const char *user, *group;
extern bool verbose;
-extern struct backend *backend;
-#define for_each_backend(b) for (b = backend; b != NULL; b = b->next)
+/* Linked list of backends. Each backend struct is followed by either
+ * a filter or plugin struct. "top" points to the first one. They
+ * are linked through the backend->next field.
+ *
+ * ┌──────────┐ ┌──────────┐ ┌──────────┐
+ * top ───▶│ backend │───▶│ backend │───▶│ backend │
+ * │ b->i = 2 │ │ b->i = 1 │ │ b->i = 0 │
+ * │ filter │ │ filter │ │ plugin │
+ * └──────────┘ └──────────┘ └──────────┘
+ */
+extern struct backend *top;
+#define for_each_backend(b) for (b = top; b != NULL; b = b->next)
/* quit.c */
extern volatile int quit;
diff --git a/server/connections.c b/server/connections.c
index a2049325..7e9584b3 100644
--- a/server/connections.c
+++ b/server/connections.c
@@ -146,23 +146,23 @@ handle_single_connection (int sockin, int sockout)
lock_connection ();
- if (backend->thread_model (backend) < NBDKIT_THREAD_MODEL_PARALLEL ||
+ if (top->thread_model (top) < NBDKIT_THREAD_MODEL_PARALLEL ||
nworkers == 1)
nworkers = 0;
conn = new_connection (sockin, sockout, nworkers);
if (!conn)
goto done;
- /* NB: because of an asynchronous exit backend can be set to NULL at
+ /* NB: because of an asynchronous exit top can be set to NULL at
* just about any time.
*/
- if (backend)
- plugin_name = backend->plugin_name (backend);
+ if (top)
+ plugin_name = top->plugin_name (top);
else
plugin_name = "(unknown)";
threadlocal_set_name (plugin_name);
- if (backend && backend->preconnect (backend, read_only) == -1)
+ if (top && top->preconnect (top, read_only) == -1)
goto done;
/* NBD handshake.
@@ -225,7 +225,7 @@ handle_single_connection (int sockin, int sockout)
/* Finalize (for filters), called just before close. */
lock_request ();
- r = backend_finalize (backend);
+ r = backend_finalize (top);
unlock_request ();
if (r == -1)
goto done;
@@ -251,12 +251,12 @@ new_connection (int sockin, int sockout, int nworkers)
conn->status_pipe[0] = conn->status_pipe[1] = -1;
- conn->handles = calloc (backend->i + 1, sizeof *conn->handles);
+ conn->handles = calloc (top->i + 1, sizeof *conn->handles);
if (conn->handles == NULL) {
perror ("malloc");
goto error;
}
- conn->nr_handles = backend->i + 1;
+ conn->nr_handles = top->i + 1;
for_each_backend (b)
reset_b_conn_handle (&conn->handles[b->i]);
@@ -277,7 +277,7 @@ new_connection (int sockin, int sockout, int nworkers)
* we aren't accepting until the plugin is not running, making
* non-atomicity okay.
*/
- assert (backend->thread_model (backend) <=
+ assert (top->thread_model (top) <=
NBDKIT_THREAD_MODEL_SERIALIZE_ALL_REQUESTS);
lock_request (NULL);
if (pipe (conn->status_pipe)) {
@@ -354,7 +354,7 @@ free_connection (struct connection *conn)
*/
if (!quit) {
lock_request ();
- backend_close (backend);
+ backend_close (top);
unlock_request ();
}
diff --git a/server/locks.c b/server/locks.c
index f005710d..6211648d 100644
--- a/server/locks.c
+++ b/server/locks.c
@@ -69,7 +69,7 @@ name_of_thread_model (int model)
void
lock_init_thread_model (void)
{
- thread_model = backend->thread_model (backend);
+ thread_model = top->thread_model (top);
debug ("using thread model: %s", name_of_thread_model (thread_model));
}
diff --git a/server/main.c b/server/main.c
index 550a8714..dbeca624 100644
--- a/server/main.c
+++ b/server/main.c
@@ -102,8 +102,8 @@ bool verbose; /* -v */
bool vsock; /* --vsock */
unsigned int socket_activation /* $LISTEN_FDS and $LISTEN_PID set */;
-/* The currently loaded plugin. */
-struct backend *backend;
+/* The linked list of zero or more filters, and one plugin. */
+struct backend *top;
static char *random_fifo_dir = NULL;
static char *random_fifo = NULL;
@@ -555,10 +555,10 @@ main (int argc, char *argv[])
/* Open the plugin (first) and then wrap the plugin with the
* filters. The filters are wrapped in reverse order that they
- * appear on the command line so that in the end ‘backend’ points to
+ * appear on the command line so that in the end ‘top’ points to
* the first filter on the command line.
*/
- backend = open_plugin_so (0, filename, short_name);
+ top = open_plugin_so (0, filename, short_name);
i = 1;
while (filter_filenames) {
struct filter_filename *t = filter_filenames;
@@ -566,7 +566,7 @@ main (int argc, char *argv[])
filename = t->filename;
short_name = is_short_name (filename);
- backend = open_filter_so (backend, i++, filename, short_name);
+ top = open_filter_so (top, i++, filename, short_name);
filter_filenames = t->next;
free (t);
@@ -586,7 +586,7 @@ main (int argc, char *argv[])
printf ("\n");
b->usage (b);
}
- backend->free (backend);
+ top->free (top);
exit (EXIT_SUCCESS);
}
@@ -601,7 +601,7 @@ main (int argc, char *argv[])
printf (" %s", v);
printf ("\n");
}
- backend->free (backend);
+ top->free (top);
exit (EXIT_SUCCESS);
}
@@ -615,16 +615,16 @@ main (int argc, char *argv[])
* first parameter is bare it is prefixed with the key "script", and
* any other bare parameters are errors.
*/
- magic_config_key = backend->magic_config_key (backend);
+ magic_config_key = top->magic_config_key (top);
for (i = 0; optind < argc; ++i, ++optind) {
p = strchr (argv[optind], '=');
if (p && is_config_key (argv[optind], p - argv[optind])) { /* key=value */
*p = '\0';
- backend->config (backend, argv[optind], p+1);
+ top->config (top, argv[optind], p+1);
}
else if (magic_config_key == NULL) {
if (i == 0) /* magic script parameter */
- backend->config (backend, "script", argv[optind]);
+ top->config (top, "script", argv[optind]);
else {
fprintf (stderr,
"%s: expecting key=value on the command line but got: %s\n",
@@ -633,7 +633,7 @@ main (int argc, char *argv[])
}
}
else { /* magic config key */
- backend->config (backend, magic_config_key, argv[optind]);
+ top->config (top, magic_config_key, argv[optind]);
}
}
@@ -643,12 +643,12 @@ main (int argc, char *argv[])
* parameters.
*/
if (dump_plugin) {
- backend->dump_fields (backend);
- backend->free (backend);
+ top->dump_fields (top);
+ top->free (top);
exit (EXIT_SUCCESS);
}
- backend->config_complete (backend);
+ top->config_complete (top);
/* Select the correct thread model based on config. */
lock_init_thread_model ();
@@ -660,8 +660,8 @@ main (int argc, char *argv[])
start_serving ();
- backend->free (backend);
- backend = NULL;
+ top->free (top);
+ top = NULL;
free (unixsocket);
free (pidfile);
diff --git a/server/plugins.c b/server/plugins.c
index 9595269c..16b4099b 100644
--- a/server/plugins.c
+++ b/server/plugins.c
@@ -144,7 +144,7 @@ plugin_dump_fields (struct backend *b)
printf ("max_thread_model=%s\n",
name_of_thread_model (p->plugin._thread_model));
printf ("thread_model=%s\n",
- name_of_thread_model (backend->thread_model (backend)));
+ name_of_thread_model (top->thread_model (top)));
printf ("errno_is_preserved=%d\n", !!p->plugin.errno_is_preserved);
if (p->plugin.magic_config_key)
printf ("magic_config_key=%s\n", p->plugin.magic_config_key);
diff --git a/server/protocol-handshake-newstyle.c b/server/protocol-handshake-newstyle.c
index 41b2a6e4..946060ac 100644
--- a/server/protocol-handshake-newstyle.c
+++ b/server/protocol-handshake-newstyle.c
@@ -478,9 +478,9 @@ negotiate_handshake_newstyle_options (void)
* disconnecting.
*/
if (finish_newstyle_options (&exportsize) == -1) {
- if (backend_finalize (backend) == -1)
+ if (backend_finalize (top) == -1)
return -1;
- backend_close (backend);
+ backend_close (top);
if (send_newstyle_option_reply (option, NBD_REP_ERR_UNKNOWN) == -1)
return -1;
continue;
@@ -518,9 +518,9 @@ negotiate_handshake_newstyle_options (void)
return -1;
if (option == NBD_OPT_INFO) {
- if (backend_finalize (backend) == -1)
+ if (backend_finalize (top) == -1)
return -1;
- backend_close (backend);
+ backend_close (top);
}
break;
diff --git a/server/protocol-handshake.c b/server/protocol-handshake.c
index a32fcde0..70ea4933 100644
--- a/server/protocol-handshake.c
+++ b/server/protocol-handshake.c
@@ -79,14 +79,14 @@ protocol_common_open (uint64_t *exportsize, uint16_t *flags)
uint16_t eflags = NBD_FLAG_HAS_FLAGS;
int fl;
- if (backend_open (backend, read_only) == -1)
+ if (backend_open (top, read_only) == -1)
return -1;
/* Prepare (for filters), called just after open. */
- if (backend_prepare (backend) == -1)
+ if (backend_prepare (top) == -1)
return -1;
- size = backend_get_size (backend);
+ size = backend_get_size (top);
if (size == -1)
return -1;
if (size < 0) {
@@ -98,57 +98,57 @@ protocol_common_open (uint64_t *exportsize, uint16_t *flags)
/* Check all flags even if they won't be advertised, to prime the
* cache and make later request validation easier.
*/
- fl = backend_can_write (backend);
+ fl = backend_can_write (top);
if (fl == -1)
return -1;
if (!fl)
eflags |= NBD_FLAG_READ_ONLY;
- fl = backend_can_zero (backend);
+ fl = backend_can_zero (top);
if (fl == -1)
return -1;
if (fl)
eflags |= NBD_FLAG_SEND_WRITE_ZEROES;
- fl = backend_can_fast_zero (backend);
+ fl = backend_can_fast_zero (top);
if (fl == -1)
return -1;
if (fl)
eflags |= NBD_FLAG_SEND_FAST_ZERO;
- fl = backend_can_trim (backend);
+ fl = backend_can_trim (top);
if (fl == -1)
return -1;
if (fl)
eflags |= NBD_FLAG_SEND_TRIM;
- fl = backend_can_fua (backend);
+ fl = backend_can_fua (top);
if (fl == -1)
return -1;
if (fl)
eflags |= NBD_FLAG_SEND_FUA;
- fl = backend_can_flush (backend);
+ fl = backend_can_flush (top);
if (fl == -1)
return -1;
if (fl)
eflags |= NBD_FLAG_SEND_FLUSH;
- fl = backend_is_rotational (backend);
+ fl = backend_is_rotational (top);
if (fl == -1)
return -1;
if (fl)
eflags |= NBD_FLAG_ROTATIONAL;
/* multi-conn is useless if parallel connections are not allowed. */
- fl = backend_can_multi_conn (backend);
+ fl = backend_can_multi_conn (top);
if (fl == -1)
return -1;
- if (fl && (backend->thread_model (backend) >
+ if (fl && (top->thread_model (top) >
NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS))
eflags |= NBD_FLAG_CAN_MULTI_CONN;
- fl = backend_can_cache (backend);
+ fl = backend_can_cache (top);
if (fl == -1)
return -1;
if (fl)
@@ -159,7 +159,7 @@ protocol_common_open (uint64_t *exportsize, uint16_t *flags)
* not have to worry about errors, and makes test-layers easier to
* write.
*/
- fl = backend_can_extents (backend);
+ fl = backend_can_extents (top);
if (fl == -1)
return -1;
diff --git a/server/protocol.c b/server/protocol.c
index d41ad569..b56d16bd 100644
--- a/server/protocol.c
+++ b/server/protocol.c
@@ -72,7 +72,7 @@ validate_request (uint16_t cmd, uint16_t flags, uint64_t offset, uint32_t count,
case NBD_CMD_TRIM:
case NBD_CMD_WRITE_ZEROES:
case NBD_CMD_BLOCK_STATUS:
- if (!backend_valid_range (backend, offset, count)) {
+ if (!backend_valid_range (top, offset, count)) {
/* XXX Allow writes to extend the disk? */
nbdkit_error ("invalid request: %s: offset and count are out of range: "
"offset=%" PRIu64 " count=%" PRIu32,
@@ -238,31 +238,31 @@ handle_request (uint16_t cmd, uint16_t flags, uint64_t offset, uint32_t count,
switch (cmd) {
case NBD_CMD_READ:
- if (backend_pread (backend, buf, count, offset, 0, &err) == -1)
+ if (backend_pread (top, buf, count, offset, 0, &err) == -1)
return err;
break;
case NBD_CMD_WRITE:
if (flags & NBD_CMD_FLAG_FUA)
f |= NBDKIT_FLAG_FUA;
- if (backend_pwrite (backend, buf, count, offset, f, &err) == -1)
+ if (backend_pwrite (top, buf, count, offset, f, &err) == -1)
return err;
break;
case NBD_CMD_FLUSH:
- if (backend_flush (backend, 0, &err) == -1)
+ if (backend_flush (top, 0, &err) == -1)
return err;
break;
case NBD_CMD_TRIM:
if (flags & NBD_CMD_FLAG_FUA)
f |= NBDKIT_FLAG_FUA;
- if (backend_trim (backend, count, offset, f, &err) == -1)
+ if (backend_trim (top, count, offset, f, &err) == -1)
return err;
break;
case NBD_CMD_CACHE:
- if (backend_cache (backend, count, offset, 0, &err) == -1)
+ if (backend_cache (top, count, offset, 0, &err) == -1)
return err;
break;
@@ -273,14 +273,14 @@ handle_request (uint16_t cmd, uint16_t flags, uint64_t offset, uint32_t count,
f |= NBDKIT_FLAG_FUA;
if (flags & NBD_CMD_FLAG_FAST_ZERO)
f |= NBDKIT_FLAG_FAST_ZERO;
- if (backend_zero (backend, count, offset, f, &err) == -1)
+ if (backend_zero (top, count, offset, f, &err) == -1)
return err;
break;
case NBD_CMD_BLOCK_STATUS:
if (flags & NBD_CMD_FLAG_REQ_ONE)
f |= NBDKIT_FLAG_REQ_ONE;
- if (backend_extents (backend, count, offset, f,
+ if (backend_extents (top, count, offset, f,
extents, &err) == -1)
return err;
break;
@@ -683,7 +683,7 @@ protocol_recv_request_send_reply (void)
/* Allocate the extents list for block status only. */
if (cmd == NBD_CMD_BLOCK_STATUS) {
- extents = nbdkit_extents_new (offset, backend_get_size (backend));
+ extents = nbdkit_extents_new (offset, backend_get_size (top));
if (extents == NULL) {
error = ENOMEM;
goto send_reply;
--
2.25.0
4 years, 9 months
- 2
- 7
[help] failed to launch guestfs in different Openstack environment
by Phoenix.Y.Wang＠dell.com
Hello guys,
This is Phoenix from Dell and we now have one strange issue related to launch guestfs in Openstack environment. In Openstack of our lab, guestfs can be launched successfully within one SLES12 instance. While when our SLES12 instance is deployed in customer's Openstack, we see guestfs cannot be launched. And also the error message is ambiguous :
[cid:e6918586-90ff-46c7-b0d3-97e48ac9a22a]
Do you have any idea whether there is some settings in Openstack ( such as KVM setting, libvirt setting ...) will prevent guestfs from launching ? Or is there any method for us to retrieve further debugging information ? We have enable debug option in guestfish command : "guestfish -x -v --ro -a ./7f489bbb-a0c3-42bc-9f55-35db948d5575?"
Thanks a lot !
Regards,
Phoenix
4 years, 9 months
- 2
- 2
[nbdkit PATCH 0/3] Make ext2 a filter
by Eric Blake
I'm impressed that I was able to whip this out in just one day of
hacking. Below, I'll include a diff between the plugin and the
filter as of patch 1, if it aids review.
Eric Blake (3):
filters: Add ext2 filter
ext2: Deprecate ext2 plugin
ext2: Add mode for letting client exportname choose file from image
TODO | 5 -
configure.ac | 8 +-
filters/ext2/Makefile.am | 75 +++++
filters/ext2/ext2.c | 415 +++++++++++++++++++++++++
filters/ext2/io.c | 466 ++++++++++++++++++++++++++++
filters/ext2/io.h | 57 ++++
filters/ext2/nbdkit-ext2-filter.pod | 102 ++++++
plugins/ext2/nbdkit-ext2-plugin.pod | 12 +-
tests/Makefile.am | 2 +-
tests/test-ext2.c | 28 +-
10 files changed, 1153 insertions(+), 17 deletions(-)
create mode 100644 filters/ext2/Makefile.am
create mode 100644 filters/ext2/ext2.c
create mode 100644 filters/ext2/io.c
create mode 100644 filters/ext2/io.h
create mode 100644 filters/ext2/nbdkit-ext2-filter.pod
{plugins => filters}/ext2/ext2.c | 173 ++++++++++++++++++++++++---------------
1 file changed, 105 insertions(+), 68 deletions(-)
diff --git a/plugins/ext2/ext2.c b/filters/ext2/ext2.c
index 6698d99f..d53743cd 100644
--- a/plugins/ext2/ext2.c
+++ b/filters/ext2/ext2.c
@@ -1,5 +1,5 @@
/* nbdkit
- * Copyright (C) 2017-2019 Red Hat Inc.
+ * Copyright (C) 2017-2020 Red Hat Inc.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are
@@ -35,6 +35,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <inttypes.h>
+#include <errno.h>
/* Inlining is broken in the ext2fs header file. Disable it by
* defining the following:
@@ -44,10 +45,12 @@
#define NBDKIT_API_VERSION 2
-#include <nbdkit-plugin.h>
+#include <nbdkit-filter.h>
-/* Disk image and filename parameters. */
-static char *disk;
+#include "cleanup.h"
+#include "io.h"
+
+/* Filename parameter. */
static char *file;
static void
@@ -59,25 +62,16 @@ ext2_load (void)
static void
ext2_unload (void)
{
- free (disk);
free (file);
}
static int
-ext2_config (const char *key, const char *value)
+ext2_config (nbdkit_next_config *next, void *nxdata,
+ const char *key, const char *value)
{
- if (strcmp (key, "disk") == 0) {
- if (disk != NULL) {
- nbdkit_error ("disk parameter specified more than once");
- return -1;
- }
- disk = nbdkit_absolute_path (value);
- if (disk == NULL)
- return -1;
- }
- else if (strcmp (key, "file") == 0) {
+ if (strcmp (key, "ext2file") == 0) {
if (file != NULL) {
- nbdkit_error ("file parameter specified more than once");
+ nbdkit_error ("ext2file parameter specified more than once");
return -1;
}
file = strdup (value);
@@ -85,20 +79,17 @@ ext2_config (const char *key, const char *value)
nbdkit_error ("strdup: %m");
return -1;
}
+ return 0;
}
- else {
- nbdkit_error ("unknown parameter '%s'", key);
- return -1;
- }
-
- return 0;
+ else
+ return next (nxdata, key, value);
}
static int
-ext2_config_complete (void)
+ext2_config_complete (nbdkit_next_config_complete *next, void *nxdata)
{
- if (disk == NULL || file == NULL) {
- nbdkit_error ("you must supply disk=<DISK> and file=<FILE> parameters "
+ if (file == NULL) {
+ nbdkit_error ("you must supply ext2file=<FILE> parameter "
"after the plugin name on the command line");
return -1;
}
@@ -108,46 +99,78 @@ ext2_config_complete (void)
return -1;
}
- return 0;
+ return next (nxdata);
}
#define ext2_config_help \
- "disk=<FILENAME> (required) Raw ext2, ext3 or ext4 filesystem.\n" \
- "file=<FILENAME> (required) File to serve inside the disk image."
+ "ext2file=<FILENAME> (required) File to serve inside the disk image."
/* The per-connection handle. */
struct handle {
ext2_filsys fs; /* Filesystem handle. */
ext2_ino_t ino; /* Inode of open file. */
ext2_file_t file; /* File handle. */
+ struct nbdkit_next next; /* "name" parameter to ext2fs_open. */
};
/* Create the per-connection handle. */
static void *
-ext2_open (int readonly)
+ext2_open (nbdkit_next_open *next, void *nxdata, int readonly)
{
struct handle *h;
+
+ /* Request write access to the underlying plugin, for journal replay. */
+ if (next (nxdata, 0) == -1)
+ return NULL;
+
+ h = calloc (1, sizeof *h);
+ if (h == NULL) {
+ nbdkit_error ("calloc: %m");
+ return NULL;
+ }
+
+ return h;
+}
+
+static int
+ext2_prepare (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle,
+ int readonly)
+{
+ struct handle *h = handle;
errcode_t err;
int fs_flags;
int file_flags;
struct ext2_inode inode;
-
- h = malloc (sizeof *h);
- if (h == NULL) {
- nbdkit_error ("malloc: %m");
- return NULL;
- }
+ int64_t r;
+ CLEANUP_FREE char *name = NULL;
fs_flags = 0;
#ifdef EXT2_FLAG_64BITS
fs_flags |= EXT2_FLAG_64BITS;
#endif
+ r = next_ops->get_size (nxdata);
+ if (r == -1)
+ return -1;
+ r = next_ops->can_write (nxdata);
+ if (r == -1)
+ return -1;
+ if (r == 0)
+ readonly = 1;
+
if (!readonly)
fs_flags |= EXT2_FLAG_RW;
- err = ext2fs_open (disk, fs_flags, 0, 0, unix_io_manager, &h->fs);
+ h->next.next_ops = next_ops;
+ h->next.nxdata = nxdata;
+ name = nbdkit_io_encode (&h->next);
+ if (!name) {
+ nbdkit_error ("nbdkit_io_encode: %m");
+ return -1;
+ }
+
+ err = ext2fs_open (name, fs_flags, 0, 0, nbdkit_io_manager, &h->fs);
if (err != 0) {
- nbdkit_error ("%s: open: %s", disk, error_message (err));
+ nbdkit_error ("open: %s", error_message (err));
goto err0;
}
@@ -158,7 +181,7 @@ ext2_open (int readonly)
err = ext2fs_namei (h->fs, EXT2_ROOT_INO, EXT2_ROOT_INO,
&file[1], &h->ino);
if (err != 0) {
- nbdkit_error ("%s: %s: namei: %s", disk, file, error_message (err));
+ nbdkit_error ("%s: namei: %s", file, error_message (err));
goto err1;
}
}
@@ -168,12 +191,11 @@ ext2_open (int readonly)
*/
err = ext2fs_read_inode (h->fs, h->ino, &inode);
if (err != 0) {
- nbdkit_error ("%s: %s: inode: %s", disk, file, error_message (err));
+ nbdkit_error ("%s: inode: %s", file, error_message (err));
goto err1;
}
if (!LINUX_S_ISREG (inode.i_mode)) {
- nbdkit_error ("%s: %s: must be a regular file in the disk image",
- disk, file);
+ nbdkit_error ("%s: must be a regular file in the disk image", file);
goto err1;
}
@@ -182,17 +204,17 @@ ext2_open (int readonly)
file_flags |= EXT2_FILE_WRITE;
err = ext2fs_file_open2 (h->fs, h->ino, NULL, file_flags, &h->file);
if (err != 0) {
- nbdkit_error ("%s: %s: open: %s", disk, file, error_message (err));
+ nbdkit_error ("%s: open: %s", file, error_message (err));
goto err1;
}
- return h;
+ return 0;
err1:
ext2fs_close (h->fs);
+ h->fs = NULL;
err0:
- free (h);
- return NULL;
+ return -1;
}
/* Free up the per-connection handle. */
@@ -201,19 +223,21 @@ ext2_close (void *handle)
{
struct handle *h = handle;
- ext2fs_file_close (h->file);
- ext2fs_close (h->fs);
+ if (h->fs) {
+ ext2fs_file_close (h->file);
+ ext2fs_close (h->fs);
+ }
free (h);
}
static int
-ext2_can_fua (void *handle)
+ext2_can_fua (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle)
{
return NBDKIT_FUA_NATIVE;
}
static int
-ext2_can_cache (void *handle)
+ext2_can_cache (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle)
{
/* Let nbdkit call pread to populate the file system cache. */
return NBDKIT_CACHE_EMULATE;
@@ -231,11 +255,14 @@ ext2_can_cache (void *handle)
* but if we allowed parallel work on those handles then we would get
* data corruption, so we need to serialize connections.
*/
-#define THREAD_MODEL NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS
+static int ext2_thread_model (void)
+{
+ return NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS;
+}
/* Get the disk size. */
static int64_t
-ext2_get_size (void *handle)
+ext2_get_size (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle)
{
struct handle *h = handle;
errcode_t err;
@@ -243,7 +270,7 @@ ext2_get_size (void *handle)
err = ext2fs_file_get_lsize (h->file, (__u64 *) &size);
if (err != 0) {
- nbdkit_error ("%s: %s: lsize: %s", disk, file, error_message (err));
+ nbdkit_error ("%s: lsize: %s", file, error_message (err));
return -1;
}
return (int64_t) size;
@@ -251,8 +278,9 @@ ext2_get_size (void *handle)
/* Read data. */
static int
-ext2_pread (void *handle, void *buf, uint32_t count, uint64_t offset,
- uint32_t flags)
+ext2_pread (struct nbdkit_next_ops *next_ops, void *nxdata,
+ void *handle, void *buf, uint32_t count, uint64_t offset,
+ uint32_t flags, int *errp)
{
struct handle *h = handle;
errcode_t err;
@@ -265,13 +293,15 @@ ext2_pread (void *handle, void *buf, uint32_t count, uint64_t offset,
*/
err = ext2fs_file_llseek (h->file, offset, EXT2_SEEK_SET, NULL);
if (err != 0) {
- nbdkit_error ("%s: %s: llseek: %s", disk, file, error_message (err));
+ nbdkit_error ("%s: llseek: %s", file, error_message (err));
+ *errp = errno;
return -1;
}
err = ext2fs_file_read (h->file, buf, (unsigned int) count, &got);
if (err != 0) {
- nbdkit_error ("%s: %s: read: %s", disk, file, error_message (err));
+ nbdkit_error ("%s: read: %s", file, error_message (err));
+ *errp = errno;
return -1;
}
@@ -285,8 +315,9 @@ ext2_pread (void *handle, void *buf, uint32_t count, uint64_t offset,
/* Write data to the file. */
static int
-ext2_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offset,
- uint32_t flags)
+ext2_pwrite (struct nbdkit_next_ops *next_ops, void *nxdata,
+ void *handle, const void *buf, uint32_t count, uint64_t offset,
+ uint32_t flags, int *errp)
{
struct handle *h = handle;
errcode_t err;
@@ -295,13 +326,15 @@ ext2_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offset,
while (count > 0) {
err = ext2fs_file_llseek (h->file, offset, EXT2_SEEK_SET, NULL);
if (err != 0) {
- nbdkit_error ("%s: %s: llseek: %s", disk, file, error_message (err));
+ nbdkit_error ("%s: llseek: %s", file, error_message (err));
+ *errp = errno;
return -1;
}
err = ext2fs_file_write (h->file, buf, (unsigned int) count, &written);
if (err != 0) {
- nbdkit_error ("%s: %s: write: %s", disk, file, error_message (err));
+ nbdkit_error ("%s: write: %s", file, error_message (err));
+ *errp = errno;
return -1;
}
@@ -313,7 +346,8 @@ ext2_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offset,
if ((flags & NBDKIT_FLAG_FUA) != 0) {
err = ext2fs_file_flush (h->file);
if (err != 0) {
- nbdkit_error ("%s: %s: flush: %s", disk, file, error_message (err));
+ nbdkit_error ("%s: flush: %s", file, error_message (err));
+ *errp = errno;
return -1;
}
}
@@ -322,14 +356,16 @@ ext2_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offset,
}
static int
-ext2_flush (void *handle, uint32_t flags)
+ext2_flush (struct nbdkit_next_ops *next_ops, void *nxdata,
+ void *handle, uint32_t flags, int *errp)
{
struct handle *h = handle;
errcode_t err;
err = ext2fs_file_flush (h->file);
if (err != 0) {
- nbdkit_error ("%s: %s: flush: %s", disk, file, error_message (err));
+ nbdkit_error ("%s: flush: %s", file, error_message (err));
+ *errp = errno;
return -1;
}
@@ -341,15 +377,17 @@ ext2_flush (void *handle, uint32_t flags)
* is very obscure.
*/
-static struct nbdkit_plugin plugin = {
+static struct nbdkit_filter filter = {
.name = "ext2",
- .version = PACKAGE_VERSION,
+ .longname = "nbdkit ext2 filter",
.load = ext2_load,
.unload = ext2_unload,
.config = ext2_config,
.config_complete = ext2_config_complete,
.config_help = ext2_config_help,
+ .thread_model = ext2_thread_model,
.open = ext2_open,
+ .prepare = ext2_prepare,
.close = ext2_close,
.can_fua = ext2_can_fua,
.can_cache = ext2_can_cache,
@@ -357,7 +395,6 @@ static struct nbdkit_plugin plugin = {
.pread = ext2_pread,
.pwrite = ext2_pwrite,
.flush = ext2_flush,
- .errno_is_preserved = 1,
};
-NBDKIT_REGISTER_PLUGIN(plugin)
+NBDKIT_REGISTER_FILTER(filter)
--
2.24.1
4 years, 9 months
- 2
- 4
[nbdkit PATCH] server: Correct logic when filter fails .prepare
by Eric Blake
If .prepare fails, we do not want to call .finalize (similar to how if
.open fails, we do not want to call .close). However, the logic in
backend_finalize was checking on the wrong condition ('was the
connection opened' rather than 'was the connection prepared'), which
led to an assertion failure.
Simple reproducer:
$ nbdkit -U - -f --filter cache --run 'qemu-io -r -c quit $nbd' sh - <<\EOF
> case $1 in get_size) echo oops >&2; exit 1 ;; *) exit 2 ;; esac
> EOF
nbdkit: sh[1]: error: /tmp/nbdkitshYvAQbz/inline-script.sh: oops
nbdkit: backend.c:206: backend_finalize: Assertion `h->state & HANDLE_CONNECTED' failed.
qemu-io: can't open device nbd:unix:/tmp/nbdkit60FUTw/socket: Failed to read option reply: Unexpected end-of-file before all bytes were read
nbdkit: nbdkit command was killed by signal 6
With this patch, the command now fails gracefully:
nbdkit: sh[1]: error: /tmp/nbdkitshie18Lp/inline-script.sh: oops
qemu-io: can't open device nbd:unix:/tmp/nbdkitqvAOlr/socket: Requested export not available
Fixes: ffa98c8d
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
Pushing this to master and stable-1.16
server/backend.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/server/backend.c b/server/backend.c
index 8bfa8525..753f5cca 100644
--- a/server/backend.c
+++ b/server/backend.c
@@ -221,15 +221,13 @@ backend_finalize (struct backend *b)
if (h->state & HANDLE_FAILED)
return -1;
- if (h->handle) {
- assert (h->state & HANDLE_CONNECTED);
+ assert (h->state & HANDLE_OPEN);
+ if (h->state & HANDLE_CONNECTED) {
if (b->finalize (b, h->handle) == -1) {
h->state |= HANDLE_FAILED;
return -1;
}
}
- else
- assert (! (h->state & HANDLE_CONNECTED));
if (b->i)
return backend_finalize (b->next);
--
2.24.1
4 years, 9 months
- 2
- 2
[common PATCH v3 0/1] options: add '--blocksize' option for C-based tools
by Mykola Ivanets
From: Nikolay Ivanets <stenavin(a)gmail.com>
v3 is just a spelling correction spotted by Eric Blake
In v2 I've moved '--blocksize' parameter description into the separate
file called blocksize-option.pod so we can include it everywhere we need
similar to key-option.pod.
https://www.redhat.com/archives/libguestfs/2020-February/msg00099.html
v1 was here:
https://www.redhat.com/archives/libguestfs/2020-February/msg00096.html
Nikolay Ivanets (1):
options: add '--blocksize' option for C-based tools
options/Makefile.am | 3 +-
options/blocksize-option.pod | 11 +++
options/options.c | 13 +++-
options/options.h | 125 +++++++++++++++++++++--------------
4 files changed, 99 insertions(+), 53 deletions(-)
create mode 100644 options/blocksize-option.pod
--
2.17.2
4 years, 9 months
- 1
- 1
[PATCH nbdkit v2 0/3] server: Remove explicit connection parameter.
by Richard W.M. Jones
v1 was here:
https://www.redhat.com/archives/libguestfs/2020-February/msg00081.html
v2 replaces
struct connection *conn = GET_CONN;
with
GET_CONN;
which sets conn implicitly and asserts that it is non-NULL.
If we actually want to test if conn is non-NULL or behave
differently, then you must use threadlocal_get_conn() instead,
and some existing uses do that.
Rich.
4 years, 9 months
- 2
- 4
[PATCH nbdkit 0/3] server: Remove explicit connection parameter.
by Richard W.M. Jones
The third patch is a large but mechanical change which gets rid of
passing around struct connection * entirely within the server,
preferring instead to reference the connection through thread-local
storage.
I hope this is a gateway to simplifying other parts of the code.
Rich.
4 years, 9 months
- 2
- 7
[PATCH v2] lib: add support for disks with 4096 bytes sector size
by Mykola Ivanets
From: Nikolay Ivanets <stenavin(a)gmail.com>
Nowadays there are hard drives and operating systems which support
"4K native" sector size. In this mode physical and logical block size
exposed to the operating system is equal to 4096 bytes.
GPT partition table (as a known example) being created in this mode will
place GPT header at LBA1 which is 4096 bytes. libguetfs is unable to
recognize partition table on such physical block devices or disk images.
The reason is that libguestfs appliance will look for a GPT header at
LBA1 which is seen at 512 byte offset.
In order to fix the issue we need a way to provide correct logical block
size for attached disks. Fortunately QEMU and libvirt already provides
a way to specify physical/logical block size per disk basis.
After discussion in a mailing list we agreed that physical block size is
rarely used and is not so important. Thus both physical and logical
block size will be set to the same value.
In this patch one more optional parameter 'blocksize' is added
to add_drive_opts API method. Valid values are 512 and 4096.
add_drive_scratch has the same optional parameter for a consistency and
testing purpose.
add-domain and add_libvirt_dom will pass logical_block_size value from
libvirt XML to add_drive_opts method.
---
generator/actions_core.ml | 26 +++++-
lib/drives.c | 38 ++++++++-
lib/guestfs-internal.h | 1 +
lib/launch-direct.c | 25 ++++++
lib/launch-libvirt.c | 18 +++++
lib/launch-uml.c | 5 ++
lib/libvirt-domain.c | 50 ++++++++++--
tests/disks/Makefile.am | 6 +-
tests/disks/test-add-drive-with-blocksize.sh | 50 ++++++++++++
tests/disks/test-qemu-drive-libvirt.xml.in | 38 ++++++++-
.../test-qemu-drive-with-blocksize-libvirt.sh | 79 +++++++++++++++++++
tmp/.gitignore | 1 +
12 files changed, 323 insertions(+), 14 deletions(-)
create mode 100755 tests/disks/test-add-drive-with-blocksize.sh
create mode 100755 tests/disks/test-qemu-drive-with-blocksize-libvirt.sh
diff --git a/generator/actions_core.ml b/generator/actions_core.ml
index cb7e8dcd0..4a715d85f 100644
--- a/generator/actions_core.ml
+++ b/generator/actions_core.ml
@@ -210,7 +210,7 @@ this function fails and the C<errno> is set to C<EINVAL>." };
{ defaults with
name = "add_drive"; added = (0, 0, 3);
- style = RErr, [String (PlainString, "filename")], [OBool "readonly"; OString "format"; OString "iface"; OString "name"; OString "label"; OString "protocol"; OStringList "server"; OString "username"; OString "secret"; OString "cachemode"; OString "discard"; OBool "copyonread"];
+ style = RErr, [String (PlainString, "filename")], [OBool "readonly"; OString "format"; OString "iface"; OString "name"; OString "label"; OString "protocol"; OStringList "server"; OString "username"; OString "secret"; OString "cachemode"; OString "discard"; OBool "copyonread"; OInt "blocksize"];
once_had_no_optargs = true;
blocking = false;
fish_alias = ["add"];
@@ -469,6 +469,16 @@ of the same area of disk.
The default is false.
+=item C<blocksize>
+
+This parameter sets the sector size of the disk. Possible values are
+C<512> (the default if the parameter is omitted) or C<4096>. Use
+C<4096> when handling an \"Advanced Format\" disk that uses 4K sector
+size (L<https://en.wikipedia.org/wiki/Advanced_Format>).
+
+Only a subset of the backends support this parameter (currently only the
+libvirt and direct backends do).
+
=back" };
{ defaults with
@@ -558,6 +568,10 @@ Disks with the E<lt>readonly/E<gt> flag are skipped.
=back
+If present, the value of C<logical_block_size> attribute of E<lt>blockio/E<gt>
+tag in libvirt XML will be passed as C<blocksize> parameter to
+C<guestfs_add_drive_opts>.
+
The other optional parameters are passed directly through to
C<guestfs_add_drive_opts>." };
@@ -597,6 +611,10 @@ The optional C<readonlydisk> parameter controls what we do for
disks which are marked E<lt>readonly/E<gt> in the libvirt XML.
See C<guestfs_add_domain> for possible values.
+If present, the value of C<logical_block_size> attribute of E<lt>blockio/E<gt>
+tag in libvirt XML will be passed as C<blocksize> parameter to
+C<guestfs_add_drive_opts>.
+
The other optional parameters are passed directly through to
C<guestfs_add_drive_opts>." };
@@ -1280,7 +1298,7 @@ function." };
{ defaults with
name = "add_drive_scratch"; added = (1, 23, 10);
- style = RErr, [Int64 "size"], [OString "name"; OString "label"];
+ style = RErr, [Int64 "size"], [OString "name"; OString "label"; OInt "blocksize"];
blocking = false;
fish_alias = ["scratch"];
shortdesc = "add a temporary scratch drive";
@@ -1290,8 +1308,8 @@ C<size> parameter is the virtual size (in bytes). The scratch
drive is blank initially (all reads return zeroes until you start
writing to it). The drive is deleted when the handle is closed.
-The optional arguments C<name> and C<label> are passed through to
-C<guestfs_add_drive>." };
+The optional arguments C<name>, C<label> and C<blocksize> are passed through to
+C<guestfs_add_drive_opts>." };
{ defaults with
name = "journal_get"; added = (1, 23, 11);
diff --git a/lib/drives.c b/lib/drives.c
index 5a8d29ab4..bba6ff74e 100644
--- a/lib/drives.c
+++ b/lib/drives.c
@@ -58,6 +58,7 @@ struct drive_create_data {
const char *cachemode;
enum discard discard;
bool copyonread;
+ int blocksize;
};
COMPILE_REGEXP (re_hostname_port, "(.*):(\\d+)$", 0)
@@ -114,6 +115,7 @@ create_drive_file (guestfs_h *g,
drv->cachemode = data->cachemode ? safe_strdup (g, data->cachemode) : NULL;
drv->discard = data->discard;
drv->copyonread = data->copyonread;
+ drv->blocksize = data->blocksize;
if (data->readonly) {
if (create_overlay (g, drv) == -1) {
@@ -150,6 +152,7 @@ create_drive_non_file (guestfs_h *g,
drv->cachemode = data->cachemode ? safe_strdup (g, data->cachemode) : NULL;
drv->discard = data->discard;
drv->copyonread = data->copyonread;
+ drv->blocksize = data->blocksize;
if (data->readonly) {
if (create_overlay (g, drv) == -1) {
@@ -501,8 +504,13 @@ guestfs_int_drive_protocol_to_string (enum drive_protocol protocol)
static char *
drive_to_string (guestfs_h *g, const struct drive *drv)
{
+ CLEANUP_FREE char *s_blocksize = NULL;
+
+ if (drv->blocksize)
+ s_blocksize = safe_asprintf (g, "%d", drv->blocksize);
+
return safe_asprintf
- (g, "%s%s%s%s protocol=%s%s%s%s%s%s%s%s%s%s%s",
+ (g, "%s%s%s%s protocol=%s%s%s%s%s%s%s%s%s%s%s%s%s",
drv->src.u.path,
drv->readonly ? " readonly" : "",
drv->src.format ? " format=" : "",
@@ -518,7 +526,9 @@ drive_to_string (guestfs_h *g, const struct drive *drv)
drv->cachemode ? : "",
drv->discard == discard_disable ? "" :
drv->discard == discard_enable ? " discard=enable" : " discard=besteffort",
- drv->copyonread ? " copyonread" : "");
+ drv->copyonread ? " copyonread" : "",
+ drv->blocksize ? " blocksize=" : "",
+ drv->blocksize ? s_blocksize : "");
}
/**
@@ -618,6 +628,17 @@ valid_port (int port)
return 1;
}
+/**
+ * Check the block size is reasonable. It can't be other then 512 or 4096.
+ */
+static int
+valid_blocksize (int blocksize)
+{
+ if (blocksize == 512 || blocksize == 4096)
+ return 1;
+ return 0;
+}
+
static int
parse_one_server (guestfs_h *g, const char *server, struct drive_server *ret)
{
@@ -767,6 +788,10 @@ guestfs_impl_add_drive_opts (guestfs_h *g, const char *filename,
optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_COPYONREAD_BITMASK
? optargs->copyonread : false;
+ data.blocksize =
+ optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_BLOCKSIZE_BITMASK
+ ? optargs->blocksize : 0;
+
if (data.readonly && data.discard == discard_enable) {
error (g, _("discard support cannot be enabled on read-only drives"));
free_drive_servers (data.servers, data.nr_servers);
@@ -796,6 +821,11 @@ guestfs_impl_add_drive_opts (guestfs_h *g, const char *filename,
free_drive_servers (data.servers, data.nr_servers);
return -1;
}
+ if (data.blocksize && !valid_blocksize (data.blocksize)) {
+ error (g, _("%s parameter is invalid"), "blocksize");
+ free_drive_servers (data.servers, data.nr_servers);
+ return -1;
+ }
if (STREQ (protocol, "file")) {
if (data.servers != NULL) {
@@ -982,6 +1012,10 @@ guestfs_impl_add_drive_scratch (guestfs_h *g, int64_t size,
add_drive_optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_LABEL_BITMASK;
add_drive_optargs.label = optargs->label;
}
+ if (optargs->bitmask & GUESTFS_ADD_DRIVE_SCRATCH_BLOCKSIZE_BITMASK) {
+ add_drive_optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_BLOCKSIZE_BITMASK;
+ add_drive_optargs.blocksize = optargs->blocksize;
+ }
/* Create the temporary file. We don't have to worry about cleanup
* because everything in g->tmpdir is 'rm -rf'd when the handle is
diff --git a/lib/guestfs-internal.h b/lib/guestfs-internal.h
index 6799c265d..8c6affa54 100644
--- a/lib/guestfs-internal.h
+++ b/lib/guestfs-internal.h
@@ -261,6 +261,7 @@ struct drive {
char *cachemode;
enum discard discard;
bool copyonread;
+ int blocksize;
};
/* Extra hv parameters (from guestfs_config). */
diff --git a/lib/launch-direct.c b/lib/launch-direct.c
index ae6ca093b..0f4bbf15f 100644
--- a/lib/launch-direct.c
+++ b/lib/launch-direct.c
@@ -273,6 +273,27 @@ add_drive_standard_params (guestfs_h *g, struct backend_direct_data *data,
return -1;
}
+/**
+ * Add the physical_block_size and logical_block_size elements of the C<-device>
+ * parameter.
+ */
+static int
+add_device_blocksize_params (guestfs_h *g, struct qemuopts *qopts,
+ struct drive *drv)
+{
+ if (drv->blocksize) {
+ append_list_format ("physical_block_size=%d", drv->blocksize);
+ append_list_format ("logical_block_size=%d", drv->blocksize);
+ }
+
+ return 0;
+
+ /* This label is called implicitly from the qemuopts macros on error. */
+ qemuopts_error:
+ perrorf (g, "qemuopts");
+ return -1;
+}
+
static int
add_drive (guestfs_h *g, struct backend_direct_data *data,
struct qemuopts *qopts, size_t i, struct drive *drv)
@@ -291,6 +312,8 @@ add_drive (guestfs_h *g, struct backend_direct_data *data,
append_list_format ("drive=hd%zu", i);
if (drv->disk_label)
append_list_format ("serial=%s", drv->disk_label);
+ if (add_device_blocksize_params (g, qopts, drv) == -1)
+ return -1;
} end_list ();
}
#if defined(__arm__) || defined(__aarch64__) || defined(__powerpc__)
@@ -317,6 +340,8 @@ add_drive (guestfs_h *g, struct backend_direct_data *data,
append_list_format ("drive=hd%zu", i);
if (drv->disk_label)
append_list_format ("serial=%s", drv->disk_label);
+ if (add_device_blocksize_params (g, qopts, drv) == -1)
+ return -1;
} end_list ();
}
diff --git a/lib/launch-libvirt.c b/lib/launch-libvirt.c
index f2cad9300..49786fdd9 100644
--- a/lib/launch-libvirt.c
+++ b/lib/launch-libvirt.c
@@ -1043,6 +1043,7 @@ static int construct_libvirt_xml_disk (guestfs_h *g, const struct backend_libvir
static int construct_libvirt_xml_disk_target (guestfs_h *g, xmlTextWriterPtr xo, size_t drv_index);
static int construct_libvirt_xml_disk_driver_qemu (guestfs_h *g, const struct backend_libvirt_data *data, struct drive *drv, xmlTextWriterPtr xo, const char *format, const char *cachemode, enum discard discard, bool copyonread);
static int construct_libvirt_xml_disk_address (guestfs_h *g, xmlTextWriterPtr xo, size_t drv_index);
+static int construct_libvirt_xml_disk_blockio (guestfs_h *g, xmlTextWriterPtr xo, int blocksize);
static int construct_libvirt_xml_disk_source_hosts (guestfs_h *g, xmlTextWriterPtr xo, const struct drive_source *src);
static int construct_libvirt_xml_disk_source_seclabel (guestfs_h *g, const struct backend_libvirt_data *data, xmlTextWriterPtr xo);
static int construct_libvirt_xml_appliance (guestfs_h *g, const struct libvirt_xml_params *params, xmlTextWriterPtr xo);
@@ -1578,6 +1579,9 @@ construct_libvirt_xml_disk (guestfs_h *g,
if (construct_libvirt_xml_disk_address (g, xo, drv_index) == -1)
return -1;
+ if (construct_libvirt_xml_disk_blockio (g, xo, drv->blocksize) == -1)
+ return -1;
+
} end_element (); /* </disk> */
return 0;
@@ -1685,6 +1689,20 @@ construct_libvirt_xml_disk_address (guestfs_h *g, xmlTextWriterPtr xo,
return 0;
}
+static int
+construct_libvirt_xml_disk_blockio (guestfs_h *g, xmlTextWriterPtr xo,
+ int blocksize)
+{
+ if (blocksize) {
+ start_element ("blockio") {
+ attribute_format ("physical_block_size", "%d", blocksize);
+ attribute_format ("logical_block_size", "%d", blocksize);
+ } end_element ();
+ }
+
+ return 0;
+}
+
static int
construct_libvirt_xml_disk_source_hosts (guestfs_h *g,
xmlTextWriterPtr xo,
diff --git a/lib/launch-uml.c b/lib/launch-uml.c
index da20c17d9..cd181b4b6 100644
--- a/lib/launch-uml.c
+++ b/lib/launch-uml.c
@@ -124,6 +124,11 @@ uml_supported (guestfs_h *g)
_("uml backend does not support drives with ‘discard’ parameter set to ‘enable’"));
return false;
}
+ if (drv->blocksize) {
+ error (g,
+ _("uml backend does not support drives with ‘blocksize’ parameter"));
+ return false;
+ }
}
return true;
diff --git a/lib/libvirt-domain.c b/lib/libvirt-domain.c
index 37c0b49b2..009a22ad6 100644
--- a/lib/libvirt-domain.c
+++ b/lib/libvirt-domain.c
@@ -42,11 +42,12 @@
#if defined(HAVE_LIBVIRT)
static xmlDocPtr get_domain_xml (guestfs_h *g, virDomainPtr dom);
-static ssize_t for_each_disk (guestfs_h *g, virConnectPtr conn, xmlDocPtr doc, int (*f) (guestfs_h *g, const char *filename, const char *format, int readonly, const char *protocol, char *const *server, const char *username, const char *secret, void *data), void *data);
+static ssize_t for_each_disk (guestfs_h *g, virConnectPtr conn, xmlDocPtr doc, int (*f) (guestfs_h *g, const char *filename, const char *format, int readonly, const char *protocol, char *const *server, const char *username, const char *secret, int blocksize, void *data), void *data);
static int libvirt_selinux_label (guestfs_h *g, xmlDocPtr doc, char **label_rtn, char **imagelabel_rtn);
static char *filename_from_pool (guestfs_h *g, virConnectPtr conn, const char *pool_nane, const char *volume_name);
static bool xpath_object_is_empty (xmlXPathObjectPtr obj);
static char *xpath_object_get_string (xmlDocPtr doc, xmlXPathObjectPtr obj);
+static int xpath_object_get_int (xmlDocPtr doc, xmlXPathObjectPtr obj);
static void
ignore_errors (void *ignore, virErrorPtr ignore2)
@@ -169,7 +170,7 @@ guestfs_impl_add_domain (guestfs_h *g, const char *domain_name,
return r;
}
-static int add_disk (guestfs_h *g, const char *filename, const char *format, int readonly, const char *protocol, char *const *server, const char *username, const char *secret, void *data);
+static int add_disk (guestfs_h *g, const char *filename, const char *format, int readonly, const char *protocol, char *const *server, const char *username, const char *secret, int blocksize, void *data);
static int connect_live (guestfs_h *g, virDomainPtr dom);
enum readonlydisk {
@@ -331,7 +332,7 @@ static int
add_disk (guestfs_h *g,
const char *filename, const char *format, int readonly_in_xml,
const char *protocol, char *const *server, const char *username,
- const char *secret, void *datavp)
+ const char *secret, int blocksize, void *datavp)
{
struct add_disk_data *data = datavp;
/* Copy whole struct so we can make local changes: */
@@ -392,6 +393,10 @@ add_disk (guestfs_h *g,
optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_SECRET_BITMASK;
optargs.secret = secret;
}
+ if (blocksize) {
+ optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_BLOCKSIZE_BITMASK;
+ optargs.blocksize = blocksize;
+ }
return guestfs_add_drive_opts_argv (g, filename, &optargs);
}
@@ -486,7 +491,7 @@ for_each_disk (guestfs_h *g,
int readonly,
const char *protocol, char *const *server,
const char *username, const char *secret,
- void *data),
+ int blocksize, void *data),
void *data)
{
size_t i, nr_added = 0, nr_nodes;
@@ -526,6 +531,7 @@ for_each_disk (guestfs_h *g,
CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xppool = NULL;
CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpvolume = NULL;
int readonly;
+ int blocksize = 0;
int t;
virErrorPtr err;
@@ -778,8 +784,17 @@ for_each_disk (guestfs_h *g,
if (!xpath_object_is_empty (xpreadonly))
readonly = 1;
+ /* Get logical block size. Optional. */
+ xpathCtx->node = nodes->nodeTab[i];
+ xpformat = xmlXPathEvalExpression (BAD_CAST
+ "./blockio/@logical_block_size",
+ xpathCtx);
+ if (!xpath_object_is_empty (xpformat))
+ blocksize = xpath_object_get_int (doc, xpformat);
+
if (f)
- t = f (g, filename, format, readonly, protocol, server, username, secret, data);
+ t = f (g, filename, format, readonly, protocol, server, username,
+ secret, blocksize, data);
else
t = 0;
@@ -975,6 +990,31 @@ xpath_object_get_string (xmlDocPtr doc, xmlXPathObjectPtr obj)
return value;
}
+/* Get the integer value from C<obj>.
+ *
+ * C<obj> is I<required> to not be empty, i.e. that C<xpath_object_is_empty>
+ * is C<false>.
+ *
+ * Any parsing errors are ignored and 0 (zero) will be returned.
+ */
+static int
+xpath_object_get_int (xmlDocPtr doc, xmlXPathObjectPtr obj)
+{
+ xmlAttrPtr attr;
+ CLEANUP_FREE char *str;
+ int value;
+
+ assert (obj->nodesetval->nodeTab[0]);
+ assert (obj->nodesetval->nodeTab[0]->type == XML_ATTRIBUTE_NODE);
+ attr = (xmlAttrPtr) obj->nodesetval->nodeTab[0];
+ str = (char *) xmlNodeListGetString (doc, attr->children, 1);
+
+ if (sscanf (str, "%d", &value) != 1)
+ value = 0; /* ignore any parsing error */
+
+ return value;
+}
+
#else /* no libvirt at compile time */
#define NOT_IMPL(r) \
diff --git a/tests/disks/Makefile.am b/tests/disks/Makefile.am
index 779871aff..bdbcccf5e 100644
--- a/tests/disks/Makefile.am
+++ b/tests/disks/Makefile.am
@@ -25,13 +25,15 @@ TESTS = \
if HAVE_LIBVIRT
TESTS += \
- test-qemu-drive-libvirt.sh
+ test-qemu-drive-libvirt.sh \
+ test-qemu-drive-with-blocksize-libvirt.sh
if ENABLE_APPLIANCE
TESTS += \
test-27-disks.sh \
test-255-disks.sh \
- test-add-lots-of-disks.sh
+ test-add-lots-of-disks.sh \
+ test-add-drive-with-blocksize.sh
endif
endif
diff --git a/tests/disks/test-add-drive-with-blocksize.sh b/tests/disks/test-add-drive-with-blocksize.sh
new file mode 100755
index 000000000..ae01fd872
--- /dev/null
+++ b/tests/disks/test-add-drive-with-blocksize.sh
@@ -0,0 +1,50 @@
+#!/bin/bash -
+# libguestfs
+# Copyright (C) 2020 Red Hat Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+# Test blocksize parameter of add-drive command
+
+set -e
+
+$TEST_FUNCTIONS
+skip_if_skipped
+
+# Test valid values
+for expected_bs in 512 4096; do
+ actual_bs=$(guestfish --ro add /dev/null blocksize:${expected_bs} : run : blockdev-getss /dev/sda)
+ if [ "${actual_bs}" != "${expected_bs}" ]; then
+ echo "$0: error: actual blocksize doesn't match expected: ${actual_bs} != ${expected_bs}"
+ exit 1
+ fi
+done
+
+# Test without blocksize parameter
+expected_bs=512
+actual_bs=$(guestfish --ro add /dev/null : run : blockdev-getss /dev/sda)
+
+if [ "${actual_bs}" != "${expected_bs}" ]; then
+ echo "$0: error: actual blocksize doesn't match expected: ${actual_bs} != ${expected_bs}"
+ exit 1
+fi
+
+# Negative tests
+for blocksize in 256 1000 2048 8192 65536; do
+ if guestfish --ro add /dev/null blocksize:${blocksize}; then
+ echo "$0: error: blocksize:${blocksize} should not be supported"
+ exit 1
+ fi
+done
diff --git a/tests/disks/test-qemu-drive-libvirt.xml.in b/tests/disks/test-qemu-drive-libvirt.xml.in
index 9b729894d..9e3eec3be 100644
--- a/tests/disks/test-qemu-drive-libvirt.xml.in
+++ b/tests/disks/test-qemu-drive-libvirt.xml.in
@@ -150,6 +150,43 @@
</devices>
</domain>
+ <domain type='test' xmlns:test='http://libvirt.org/schemas/domain/test/1.0'>
+ <test:runstate>5</test:runstate> <!-- 5 == VIR_DOMAIN_SHUTOFF -->
+ <name>blocksize</name>
+ <memory>1048576</memory>
+ <os>
+ <type>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <devices>
+ <disk type='block' device='disk'>
+ <driver name='qemu' type='raw'/>
+ <source dev='/dev/sda'/>
+ <target dev='vda' bus='virtio'/>
+ </disk>
+ <disk type='block' device='disk'>
+ <driver name='qemu' type='raw'/>
+ <source dev='/dev/sdb'/>
+ <blockio logical_block_size='512'/>
+ <target dev='vdb' bus='virtio'/>
+ </disk>
+ <disk type='block' device='disk'>
+ <driver name='qemu' type='raw'/>
+ <source dev='/dev/sdc'/>
+ <blockio logical_block_size='4096'/>
+ <target dev='vdc' bus='virtio'/>
+ </disk>
+ <disk type='network' device='disk'>
+ <driver name='qemu'/>
+ <source protocol='nbd'>
+ <host name='1.2.3.4' port='1234'/>
+ </source>
+ <blockio physical_block_size='4096' logical_block_size='512'/>
+ <target dev='vdd' bus='virtio'/>
+ </disk>
+ </devices>
+ </domain>
+
<pool type='dir'>
<name>pool1</name>
<uuid>12345678-1234-1234-1234-1234567890ab</uuid>
@@ -167,7 +204,6 @@
<path>@abs_builddir@/tmp/in-pool</path>
</target>
</volume>
-
</pool>
</node>
diff --git a/tests/disks/test-qemu-drive-with-blocksize-libvirt.sh b/tests/disks/test-qemu-drive-with-blocksize-libvirt.sh
new file mode 100755
index 000000000..1426a9e20
--- /dev/null
+++ b/tests/disks/test-qemu-drive-with-blocksize-libvirt.sh
@@ -0,0 +1,79 @@
+#!/bin/bash
+# Copyright (C) 2013-2019 Red Hat Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+# Test that disks with <blockio .../> tag map to the correct qemu -device
+# parameters and respect to logical_block_size value.
+
+set -e
+
+$TEST_FUNCTIONS
+skip_if_skipped
+skip_unless_libvirt_minimum_version 1 1 3
+
+guestfish="guestfish -c test://$abs_builddir/test-qemu-drive-libvirt.xml"
+
+export LIBGUESTFS_BACKEND=direct
+export LIBGUESTFS_HV="${abs_srcdir}/debug-qemu.sh"
+export DEBUG_QEMU_FILE="${abs_builddir}/test-qemu-drive-with-blocksize-libvirt.out"
+
+function check_output ()
+{
+ if [ ! -f "$DEBUG_QEMU_FILE" ]; then
+ echo "$0: guestfish command failed, see previous error messages"
+ exit 1
+ fi
+}
+
+function fail ()
+{
+ echo "$0: Test $1 failed. Command line output was:"
+ cat "$DEBUG_QEMU_FILE"
+ exit 1
+}
+
+# arg1 - is device number
+function find_device()
+{
+ grep -shoe "-device \S*drive=hd${1}\S*" "$DEBUG_QEMU_FILE"
+}
+
+# arg1 - is device number
+# arg2 - is expected blocksize
+function check_blocksize_for_device()
+{
+ find_device ${1} | grep -sqEe "((physical|logical)_block_size=${2},?){2}" || fail hd${1}
+}
+
+rm -f "$DEBUG_QEMU_FILE"
+
+LIBGUESTFS_DEBUG=1 $guestfish -d blocksize run ||:
+check_output
+
+# hd0 without explicitly specified physical/logical block size.
+# We don't expect neither physical_ nor logical_block_size parameter.
+find_device 0 | grep -sqhve '_block_size' || fail hd0
+
+# hd1 with logical_block_size='512'.
+check_blocksize_for_device 1 512
+
+# hd2 with logical_block_size='4096'.
+check_blocksize_for_device 2 4096
+
+# hd3 with physical_block_size='4096' logical_block_size='512'.
+check_blocksize_for_device 3 512
+
+rm -f "$DEBUG_QEMU_FILE"
diff --git a/tmp/.gitignore b/tmp/.gitignore
index 912a946b6..ff989cc68 100644
--- a/tmp/.gitignore
+++ b/tmp/.gitignore
@@ -1,6 +1,7 @@
/.guestfs-*
/guestfs.*
/libguestfs??????/
+/testdisk??????
/run-*
/v2vovl*.qcow2
/valgrind-*.log
--
2.17.2
4 years, 9 months
- 3
- 3
[nbdkit PATCH] filters: Make nxdata persistent
by Eric Blake
Future patches want to allow a filter to pass a single opaque
parameter into another framework (such as ext2fs_open) or even spawn a
helper thread, which requires that nxdata be stable for the entire
life of the connection between .open and .close. Our current approach
of creating a stack-allocated nxdata for every call does not play
nicely with that scheme, so rework things into using a malloc'd
pointer. In fact, if we use struct b_conn as our filter handle, and
merely add an additional field to track the user's handle, then we get
the long-term persistance we desire.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
docs/nbdkit-filter.pod | 7 +-
server/filters.c | 141 ++++++++++++++++++++++++++---------------
2 files changed, 96 insertions(+), 52 deletions(-)
diff --git a/docs/nbdkit-filter.pod b/docs/nbdkit-filter.pod
index 55dfab1..5fed7ca 100644
--- a/docs/nbdkit-filter.pod
+++ b/docs/nbdkit-filter.pod
@@ -131,7 +131,12 @@ C<nbdkit_next_config_complete>, C<nbdkit_next_preconnect>,
C<nbdkit_next_open>) and a structure called C<struct nbdkit_next_ops>.
These abstract the next plugin or filter in the chain. There is also
an opaque pointer C<nxdata> which must be passed along when calling
-these functions.
+these functions. The value of C<nxdata> passed to C<.open> has a
+stable lifetime that lasts to the corresponding C<.close>; with all
+intermediate functions (such ase C<.pread>) receiving the same value
+for convenience; the only exceptions where C<nxdata> is not reused are
+C<.config>, C<.config_complete>, and C<.preconnect>, which are called
+outside the lifetime of a connection.
=head2 Next config, open and close
diff --git a/server/filters.c b/server/filters.c
index ed026f5..2f65818 100644
--- a/server/filters.c
+++ b/server/filters.c
@@ -1,5 +1,5 @@
/* nbdkit
- * Copyright (C) 2013-2019 Red Hat Inc.
+ * Copyright (C) 2013-2020 Red Hat Inc.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are
@@ -49,12 +49,14 @@ struct backend_filter {
struct nbdkit_filter filter;
};
-/* Literally a backend + a connection pointer. This is the
- * implementation of ‘void *nxdata’ in the filter API.
+/* Literally a backend, a connection pointer, and the filter's handle.
+ * This is the implementation of our handle in .open, and serves as
+ * a stable ‘void *nxdata’ in the filter API.
*/
struct b_conn {
struct backend *b;
struct connection *conn;
+ void *handle;
};
/* Note this frees the whole chain. */
@@ -223,26 +225,44 @@ static void *
filter_open (struct backend *b, struct connection *conn, int readonly)
{
struct backend_filter *f = container_of (b, struct backend_filter, backend);
- struct b_conn nxdata = { .b = b->next, .conn = conn };
+ struct b_conn *nxdata = malloc (sizeof *nxdata);
+
+ if (!nxdata) {
+ nbdkit_error ("malloc: %m");
+ return NULL;
+ }
+
+ nxdata->b = b->next;
+ nxdata->conn = conn;
+ nxdata->handle = NULL;
/* Most filters will call next_open first, resulting in
* inner-to-outer ordering.
*/
if (f->filter.open)
- return f->filter.open (next_open, &nxdata, readonly);
+ nxdata->handle = f->filter.open (next_open, nxdata, readonly);
else if (backend_open (b->next, conn, readonly) == -1)
- return NULL;
+ nxdata->handle = NULL;
else
- return NBDKIT_HANDLE_NOT_NEEDED;
+ nxdata->handle = NBDKIT_HANDLE_NOT_NEEDED;
+ if (nxdata->handle == NULL) {
+ free (nxdata);
+ return NULL;
+ }
+ return nxdata;
}
static void
filter_close (struct backend *b, struct connection *conn, void *handle)
{
struct backend_filter *f = container_of (b, struct backend_filter, backend);
+ struct b_conn *nxdata = handle;
- if (handle && f->filter.close)
- f->filter.close (handle);
+ if (handle && f->filter.close) {
+ assert (nxdata->b == b->next && nxdata->conn == conn);
+ f->filter.close (nxdata->handle);
+ free (nxdata);
+ }
}
/* The next_functions structure contains pointers to backend
@@ -421,10 +441,11 @@ filter_prepare (struct backend *b, struct connection *conn, void *handle,
int readonly)
{
struct backend_filter *f = container_of (b, struct backend_filter, backend);
- struct b_conn nxdata = { .b = b->next, .conn = conn };
+ struct b_conn *nxdata = handle;
+ assert (nxdata->b == b->next && nxdata->conn == conn);
if (f->filter.prepare &&
- f->filter.prepare (&next_ops, &nxdata, handle, readonly) == -1)
+ f->filter.prepare (&next_ops, nxdata, nxdata->handle, readonly) == -1)
return -1;
return 0;
@@ -434,10 +455,11 @@ static int
filter_finalize (struct backend *b, struct connection *conn, void *handle)
{
struct backend_filter *f = container_of (b, struct backend_filter, backend);
- struct b_conn nxdata = { .b = b->next, .conn = conn };
+ struct b_conn *nxdata = handle;
+ assert (nxdata->b == b->next && nxdata->conn == conn);
if (f->filter.finalize &&
- f->filter.finalize (&next_ops, &nxdata, handle) == -1)
+ f->filter.finalize (&next_ops, nxdata, nxdata->handle) == -1)
return -1;
return 0;
}
@@ -446,10 +468,11 @@ static int64_t
filter_get_size (struct backend *b, struct connection *conn, void *handle)
{
struct backend_filter *f = container_of (b, struct backend_filter, backend);
- struct b_conn nxdata = { .b = b->next, .conn = conn };
+ struct b_conn *nxdata = handle;
+ assert (nxdata->b == b->next && nxdata->conn == conn);
if (f->filter.get_size)
- return f->filter.get_size (&next_ops, &nxdata, handle);
+ return f->filter.get_size (&next_ops, nxdata, nxdata->handle);
else
return backend_get_size (b->next, conn);
}
@@ -458,10 +481,11 @@ static int
filter_can_write (struct backend *b, struct connection *conn, void *handle)
{
struct backend_filter *f = container_of (b, struct backend_filter, backend);
- struct b_conn nxdata = { .b = b->next, .conn = conn };
+ struct b_conn *nxdata = handle;
+ assert (nxdata->b == b->next && nxdata->conn == conn);
if (f->filter.can_write)
- return f->filter.can_write (&next_ops, &nxdata, handle);
+ return f->filter.can_write (&next_ops, nxdata, nxdata->handle);
else
return backend_can_write (b->next, conn);
}
@@ -470,10 +494,11 @@ static int
filter_can_flush (struct backend *b, struct connection *conn, void *handle)
{
struct backend_filter *f = container_of (b, struct backend_filter, backend);
- struct b_conn nxdata = { .b = b->next, .conn = conn };
+ struct b_conn *nxdata = handle;
+ assert (nxdata->b == b->next && nxdata->conn == conn);
if (f->filter.can_flush)
- return f->filter.can_flush (&next_ops, &nxdata, handle);
+ return f->filter.can_flush (&next_ops, nxdata, nxdata->handle);
else
return backend_can_flush (b->next, conn);
}
@@ -482,10 +507,11 @@ static int
filter_is_rotational (struct backend *b, struct connection *conn, void *handle)
{
struct backend_filter *f = container_of (b, struct backend_filter, backend);
- struct b_conn nxdata = { .b = b->next, .conn = conn };
+ struct b_conn *nxdata = handle;
+ assert (nxdata->b == b->next && nxdata->conn == conn);
if (f->filter.is_rotational)
- return f->filter.is_rotational (&next_ops, &nxdata, handle);
+ return f->filter.is_rotational (&next_ops, nxdata, nxdata->handle);
else
return backend_is_rotational (b->next, conn);
}
@@ -494,10 +520,11 @@ static int
filter_can_trim (struct backend *b, struct connection *conn, void *handle)
{
struct backend_filter *f = container_of (b, struct backend_filter, backend);
- struct b_conn nxdata = { .b = b->next, .conn = conn };
+ struct b_conn *nxdata = handle;
+ assert (nxdata->b == b->next && nxdata->conn == conn);
if (f->filter.can_trim)
- return f->filter.can_trim (&next_ops, &nxdata, handle);
+ return f->filter.can_trim (&next_ops, nxdata, nxdata->handle);
else
return backend_can_trim (b->next, conn);
}
@@ -506,10 +533,11 @@ static int
filter_can_zero (struct backend *b, struct connection *conn, void *handle)
{
struct backend_filter *f = container_of (b, struct backend_filter, backend);
- struct b_conn nxdata = { .b = b->next, .conn = conn };
+ struct b_conn *nxdata = handle;
+ assert (nxdata->b == b->next && nxdata->conn == conn);
if (f->filter.can_zero)
- return f->filter.can_zero (&next_ops, &nxdata, handle);
+ return f->filter.can_zero (&next_ops, nxdata, nxdata->handle);
else
return backend_can_zero (b->next, conn);
}
@@ -518,10 +546,11 @@ static int
filter_can_fast_zero (struct backend *b, struct connection *conn, void *handle)
{
struct backend_filter *f = container_of (b, struct backend_filter, backend);
- struct b_conn nxdata = { .b = b->next, .conn = conn };
+ struct b_conn *nxdata = handle;
+ assert (nxdata->b == b->next && nxdata->conn == conn);
if (f->filter.can_fast_zero)
- return f->filter.can_fast_zero (&next_ops, &nxdata, handle);
+ return f->filter.can_fast_zero (&next_ops, nxdata, nxdata->handle);
else
return backend_can_fast_zero (b->next, conn);
}
@@ -530,10 +559,11 @@ static int
filter_can_extents (struct backend *b, struct connection *conn, void *handle)
{
struct backend_filter *f = container_of (b, struct backend_filter, backend);
- struct b_conn nxdata = { .b = b->next, .conn = conn };
+ struct b_conn *nxdata = handle;
+ assert (nxdata->b == b->next && nxdata->conn == conn);
if (f->filter.can_extents)
- return f->filter.can_extents (&next_ops, &nxdata, handle);
+ return f->filter.can_extents (&next_ops, nxdata, nxdata->handle);
else
return backend_can_extents (b->next, conn);
}
@@ -542,10 +572,11 @@ static int
filter_can_fua (struct backend *b, struct connection *conn, void *handle)
{
struct backend_filter *f = container_of (b, struct backend_filter, backend);
- struct b_conn nxdata = { .b = b->next, .conn = conn };
+ struct b_conn *nxdata = handle;
+ assert (nxdata->b == b->next && nxdata->conn == conn);
if (f->filter.can_fua)
- return f->filter.can_fua (&next_ops, &nxdata, handle);
+ return f->filter.can_fua (&next_ops, nxdata, nxdata->handle);
else
return backend_can_fua (b->next, conn);
}
@@ -554,10 +585,11 @@ static int
filter_can_multi_conn (struct backend *b, struct connection *conn, void *handle)
{
struct backend_filter *f = container_of (b, struct backend_filter, backend);
- struct b_conn nxdata = { .b = b->next, .conn = conn };
+ struct b_conn *nxdata = handle;
+ assert (nxdata->b == b->next && nxdata->conn == conn);
if (f->filter.can_multi_conn)
- return f->filter.can_multi_conn (&next_ops, &nxdata, handle);
+ return f->filter.can_multi_conn (&next_ops, nxdata, nxdata->handle);
else
return backend_can_multi_conn (b->next, conn);
}
@@ -566,10 +598,11 @@ static int
filter_can_cache (struct backend *b, struct connection *conn, void *handle)
{
struct backend_filter *f = container_of (b, struct backend_filter, backend);
- struct b_conn nxdata = { .b = b->next, .conn = conn };
+ struct b_conn *nxdata = handle;
+ assert (nxdata->b == b->next && nxdata->conn == conn);
if (f->filter.can_cache)
- return f->filter.can_cache (&next_ops, &nxdata, handle);
+ return f->filter.can_cache (&next_ops, nxdata, nxdata->handle);
else
return backend_can_cache (b->next, conn);
}
@@ -580,10 +613,11 @@ filter_pread (struct backend *b, struct connection *conn, void *handle,
uint32_t flags, int *err)
{
struct backend_filter *f = container_of (b, struct backend_filter, backend);
- struct b_conn nxdata = { .b = b->next, .conn = conn };
+ struct b_conn *nxdata = handle;
+ assert (nxdata->b == b->next && nxdata->conn == conn);
if (f->filter.pread)
- return f->filter.pread (&next_ops, &nxdata, handle,
+ return f->filter.pread (&next_ops, nxdata, nxdata->handle,
buf, count, offset, flags, err);
else
return backend_pread (b->next, conn, buf, count, offset, flags, err);
@@ -595,10 +629,11 @@ filter_pwrite (struct backend *b, struct connection *conn, void *handle,
uint32_t flags, int *err)
{
struct backend_filter *f = container_of (b, struct backend_filter, backend);
- struct b_conn nxdata = { .b = b->next, .conn = conn };
+ struct b_conn *nxdata = handle;
+ assert (nxdata->b == b->next && nxdata->conn == conn);
if (f->filter.pwrite)
- return f->filter.pwrite (&next_ops, &nxdata, handle,
+ return f->filter.pwrite (&next_ops, nxdata, nxdata->handle,
buf, count, offset, flags, err);
else
return backend_pwrite (b->next, conn, buf, count, offset, flags, err);
@@ -609,10 +644,11 @@ filter_flush (struct backend *b, struct connection *conn, void *handle,
uint32_t flags, int *err)
{
struct backend_filter *f = container_of (b, struct backend_filter, backend);
- struct b_conn nxdata = { .b = b->next, .conn = conn };
+ struct b_conn *nxdata = handle;
+ assert (nxdata->b == b->next && nxdata->conn == conn);
if (f->filter.flush)
- return f->filter.flush (&next_ops, &nxdata, handle, flags, err);
+ return f->filter.flush (&next_ops, nxdata, nxdata->handle, flags, err);
else
return backend_flush (b->next, conn, flags, err);
}
@@ -623,11 +659,12 @@ filter_trim (struct backend *b, struct connection *conn, void *handle,
uint32_t flags, int *err)
{
struct backend_filter *f = container_of (b, struct backend_filter, backend);
- struct b_conn nxdata = { .b = b->next, .conn = conn };
+ struct b_conn *nxdata = handle;
+ assert (nxdata->b == b->next && nxdata->conn == conn);
if (f->filter.trim)
- return f->filter.trim (&next_ops, &nxdata, handle, count, offset, flags,
- err);
+ return f->filter.trim (&next_ops, nxdata, nxdata->handle, count, offset,
+ flags, err);
else
return backend_trim (b->next, conn, count, offset, flags, err);
}
@@ -637,10 +674,11 @@ filter_zero (struct backend *b, struct connection *conn, void *handle,
uint32_t count, uint64_t offset, uint32_t flags, int *err)
{
struct backend_filter *f = container_of (b, struct backend_filter, backend);
- struct b_conn nxdata = { .b = b->next, .conn = conn };
+ struct b_conn *nxdata = handle;
+ assert (nxdata->b == b->next && nxdata->conn == conn);
if (f->filter.zero)
- return f->filter.zero (&next_ops, &nxdata, handle,
+ return f->filter.zero (&next_ops, nxdata, nxdata->handle,
count, offset, flags, err);
else
return backend_zero (b->next, conn, count, offset, flags, err);
@@ -652,10 +690,11 @@ filter_extents (struct backend *b, struct connection *conn, void *handle,
struct nbdkit_extents *extents, int *err)
{
struct backend_filter *f = container_of (b, struct backend_filter, backend);
- struct b_conn nxdata = { .b = b->next, .conn = conn };
+ struct b_conn *nxdata = handle;
+ assert (nxdata->b == b->next && nxdata->conn == conn);
if (f->filter.extents)
- return f->filter.extents (&next_ops, &nxdata, handle,
+ return f->filter.extents (&next_ops, nxdata, nxdata->handle,
count, offset, flags,
extents, err);
else
@@ -669,11 +708,11 @@ filter_cache (struct backend *b, struct connection *conn, void *handle,
uint32_t flags, int *err)
{
struct backend_filter *f = container_of (b, struct backend_filter, backend);
- struct b_conn nxdata = { .b = b->next, .conn = conn };
-
+ struct b_conn *nxdata = handle;
+ assert (nxdata->b == b->next && nxdata->conn == conn);
if (f->filter.cache)
- return f->filter.cache (&next_ops, &nxdata, handle,
+ return f->filter.cache (&next_ops, nxdata, nxdata->handle,
count, offset, flags, err);
else
return backend_cache (b->next, conn, count, offset, flags, err);
--
2.24.1
4 years, 9 months
- 2
- 1
nbdkit background threads
by Richard W.M. Jones
https://github.com/libguestfs/nbdkit/blob/ecef5b16359fb5af7e7abf4fd2fb3ad...
Already existing filters (readahead, cache) could be improved if
filters could open a background work thread or threads which connect
independently to the plugin. A proposed new filter (scan) cannot
really be written at all without this capability.
First of all the reason this can't be done today is because filters
are called with a next_ops structure which is only valid transiently
during the filter callback. It cannot safely be saved or passed to
another thread.
(https://github.com/libguestfs/nbdkit/blob/master/docs/nbdkit-filter.pod#n...)
The seemingly obvious implementation - which is what I tried today -
would be to let filters create background threads in .config_complete.
We would provide a filter API something like:
int nbdkit_open_connection (struct nbdkit_next_ops **next_ops,
void **nxdata,
/* needs a close function */);
It would return a next_ops and nxdata that the filter could then use
to make data calls from the background thread into the underlying
layers.
To ensure safe unloading of filters and plugins we would also need a
new filter callback (which I called .bg_kill) which must close all
background connections opened by the filter before returning.
I believe from my rough implementation that this is feasible. However
I also thought about another way we might do this: We might open a
loopback socket (eg. socketpair) which is passed to the filter and
which the filter connects to using libnbd. nbdkit internally would
treat this as if it was a regular external connection. Of course this
would require libnbd as a dependency, or disable filters / features if
not available.
Any thoughts on this?
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
4 years, 9 months
- 2
- 4