Richard W.M. Jones
2022-Feb-16 16:20 UTC
[Libguestfs] [PATCH nbdkit 1/6] server: Add new plugin/filter .block_size callback
This commit implements backend_block_size() and wires it up to a new
.block_size callback from the plugin through any filters on top.
This callback will allow plugins to pass their minimum, preferred and
maximum block size through the NBD protocol NBD_INFO_BLOCK_SIZE info
option.
The new function is not called from anywhere yet, so for the moment
this commit does nothing.
---
include/nbdkit-filter.h | 4 ++++
include/nbdkit-plugin.h | 3 +++
server/internal.h | 9 ++++++++
server/backend.c | 29 +++++++++++++++++++++++
server/filters.c | 17 ++++++++++++++
server/plugins.c | 51 +++++++++++++++++++++++++++++++++++++++++
6 files changed, 113 insertions(+)
diff --git a/include/nbdkit-filter.h b/include/nbdkit-filter.h
index 8b587d70..4c620b4f 100644
--- a/include/nbdkit-filter.h
+++ b/include/nbdkit-filter.h
@@ -84,6 +84,8 @@ struct nbdkit_next_ops {
/* These callbacks are the same as normal plugin operations. */
int64_t (*get_size) (nbdkit_next *nxdata);
const char * (*export_description) (nbdkit_next *nxdata);
+ int (*block_size) (nbdkit_next *nxdata,
+ uint32_t *minimum, uint32_t *preferred, uint32_t
*maximum);
int (*can_write) (nbdkit_next *nxdata);
int (*can_flush) (nbdkit_next *nxdata);
@@ -219,6 +221,8 @@ struct nbdkit_filter {
int64_t (*get_size) (nbdkit_next *next,
void *handle);
const char * (*export_description) (nbdkit_next *next, void *handle);
+ int (*block_size) (nbdkit_next *next, void *handle,
+ uint32_t *minimum, uint32_t *preferred, uint32_t
*maximum);
int (*can_write) (nbdkit_next *next,
void *handle);
diff --git a/include/nbdkit-plugin.h b/include/nbdkit-plugin.h
index 59ec11b6..50ff93df 100644
--- a/include/nbdkit-plugin.h
+++ b/include/nbdkit-plugin.h
@@ -146,6 +146,9 @@ struct nbdkit_plugin {
const char * (*export_description) (void *handle);
void (*cleanup) (void);
+
+ int (*block_size) (void *handle,
+ uint32_t *minimum, uint32_t *preferred, uint32_t
*maximum);
};
NBDKIT_EXTERN_DECL (void, nbdkit_set_error, (int err));
diff --git a/server/internal.h b/server/internal.h
index f4843164..c91d196e 100644
--- a/server/internal.h
+++ b/server/internal.h
@@ -216,6 +216,9 @@ struct context {
unsigned char state; /* Bitmask of HANDLE_* values */
uint64_t exportsize;
+ uint32_t minimum_block_size;
+ uint32_t preferred_block_size;
+ uint32_t maximum_block_size;
int can_write;
int can_flush;
int is_rotational;
@@ -368,6 +371,8 @@ struct backend {
const char *(*export_description) (struct context *);
int64_t (*get_size) (struct context *);
+ int (*block_size) (struct context *,
+ uint32_t *minimum, uint32_t *preferred, uint32_t
*maximum);
int (*can_write) (struct context *);
int (*can_flush) (struct context *);
int (*is_rotational) (struct context *);
@@ -433,6 +438,10 @@ extern const char *backend_export_description (struct
context *c)
__attribute__((__nonnull__ (1)));
extern int64_t backend_get_size (struct context *c)
__attribute__((__nonnull__ (1)));
+extern int backend_block_size (struct context *c,
+ uint32_t *minimum, uint32_t *preferred,
+ uint32_t *maximum)
+ __attribute__((__nonnull__ (1, 2, 3, 4)));
extern int backend_can_write (struct context *c)
__attribute__((__nonnull__ (1)));
extern int backend_can_flush (struct context *c)
diff --git a/server/backend.c b/server/backend.c
index 1bb3dac9..7d46b99d 100644
--- a/server/backend.c
+++ b/server/backend.c
@@ -214,6 +214,7 @@ static struct nbdkit_next_ops next_ops = {
.finalize = backend_finalize,
.export_description = backend_export_description,
.get_size = backend_get_size,
+ .block_size = backend_block_size,
.can_write = backend_can_write,
.can_flush = backend_can_flush,
.is_rotational = backend_is_rotational,
@@ -265,6 +266,7 @@ backend_open (struct backend *b, int readonly, const char
*exportname,
c->conn = shared ? NULL : conn;
c->state = 0;
c->exportsize = -1;
+ c->minimum_block_size = c->preferred_block_size =
c->maximum_block_size = -1;
c->can_write = readonly ? 0 : -1;
c->can_flush = -1;
c->is_rotational = -1;
@@ -418,6 +420,33 @@ backend_get_size (struct context *c)
return c->exportsize;
}
+int
+backend_block_size (struct context *c,
+ uint32_t *minimum, uint32_t *preferred, uint32_t *maximum)
+{
+ PUSH_CONTEXT_FOR_SCOPE (c);
+ struct backend *b = c->b;
+ int r;
+
+ assert (c->handle && (c->state & HANDLE_CONNECTED));
+ if (c->minimum_block_size != -1) {
+ *minimum = c->minimum_block_size;
+ *preferred = c->preferred_block_size;
+ *maximum = c->maximum_block_size;
+ return 0;
+ }
+ else {
+ controlpath_debug ("%s: block_size", b->name);
+ r = b->block_size (c, minimum, preferred, maximum);
+ if (r == 0) {
+ c->minimum_block_size = *minimum;
+ c->preferred_block_size = *preferred;
+ c->maximum_block_size = *maximum;
+ }
+ return r;
+ }
+}
+
int
backend_can_write (struct context *c)
{
diff --git a/server/filters.c b/server/filters.c
index 93ee55f0..ae7e1a56 100644
--- a/server/filters.c
+++ b/server/filters.c
@@ -358,6 +358,22 @@ filter_get_size (struct context *c)
return backend_get_size (c_next);
}
+static int
+filter_block_size (struct context *c,
+ uint32_t *minimum, uint32_t *preferred, uint32_t *maximum)
+{
+ struct backend *b = c->b;
+ struct backend_filter *f = container_of (b, struct backend_filter, backend);
+ struct context *c_next = c->c_next;
+
+ if (f->filter.block_size)
+ return f->filter.block_size (c_next, c->handle,
+ minimum, preferred, maximum);
+ else
+ return backend_block_size (c_next,
+ minimum, preferred, maximum);
+}
+
static int
filter_can_write (struct context *c)
{
@@ -621,6 +637,7 @@ static struct backend filter_functions = {
.close = filter_close,
.export_description = filter_export_description,
.get_size = filter_get_size,
+ .block_size = filter_block_size,
.can_write = filter_can_write,
.can_flush = filter_can_flush,
.is_rotational = filter_is_rotational,
diff --git a/server/plugins.c b/server/plugins.c
index b633c107..34d7a10c 100644
--- a/server/plugins.c
+++ b/server/plugins.c
@@ -46,6 +46,7 @@
#include "internal.h"
#include "minmax.h"
+#include "ispowerof2.h"
/* We extend the generic backend struct with extra fields relating
* to this plugin.
@@ -173,6 +174,7 @@ plugin_dump_fields (struct backend *b)
HAS (close);
HAS (export_description);
HAS (get_size);
+ HAS (block_size);
HAS (can_write);
HAS (can_flush);
HAS (is_rotational);
@@ -408,6 +410,54 @@ plugin_get_size (struct context *c)
return p->plugin.get_size (c->handle);
}
+static int
+plugin_block_size (struct context *c,
+ uint32_t *minimum, uint32_t *preferred, uint32_t *maximum)
+{
+ struct backend *b = c->b;
+ struct backend_plugin *p = container_of (b, struct backend_plugin, backend);
+ int r;
+
+ if (p->plugin.block_size) {
+ r = p->plugin.block_size (c->handle, minimum, preferred, maximum);
+ if (r == 0) {
+ /* To make scripting easier, it's permitted to set
+ * minimum = preferred = maximum = 0 and return 0.
+ * That means "no information", and works the same
+ * way as the else clause below.
+ */
+ if (*minimum == 0 && *preferred == 0 && *maximum == 0)
+ return 0;
+
+ if (*minimum < 1) {
+ nbdkit_error ("plugin must set minimum block size >= 1");
+ r = -1;
+ }
+ if (! is_power_of_2 (*minimum)) {
+ nbdkit_error ("plugin must set minimum block size to a power of
2");
+ r = -1;
+ }
+ if (*minimum > *preferred || *preferred > *maximum) {
+ nbdkit_error ("plugin must set minimum block size "
+ "<= preferred <= maximum");
+ r = -1;
+ }
+ /* XXX Other checks should be performed here:
+ * preferred and maximum should be multiples of the minimum.
+ */
+ }
+ return r;
+ }
+ else {
+ /* If there is no .block_size call then return minimum = preferred
+ * = maximum = 0, which is a sentinel meaning don't send the
+ * NBD_INFO_BLOCK_SIZE message.
+ */
+ *minimum = *preferred = *maximum = 0;
+ return 0;
+ }
+}
+
static int
normalize_bool (int value)
{
@@ -824,6 +874,7 @@ static struct backend plugin_functions = {
.close = plugin_close,
.export_description = plugin_export_description,
.get_size = plugin_get_size,
+ .block_size = plugin_block_size,
.can_write = plugin_can_write,
.can_flush = plugin_can_flush,
.is_rotational = plugin_is_rotational,
--
2.35.1
Eric Blake
2022-Feb-16 17:16 UTC
[Libguestfs] [PATCH nbdkit 1/6] server: Add new plugin/filter .block_size callback
On Wed, Feb 16, 2022 at 04:20:36PM +0000, Richard W.M. Jones wrote:> This commit implements backend_block_size() and wires it up to a new > .block_size callback from the plugin through any filters on top. > > This callback will allow plugins to pass their minimum, preferred and > maximum block size through the NBD protocol NBD_INFO_BLOCK_SIZE info > option. > > The new function is not called from anywhere yet, so for the moment > this commit does nothing. > --- > include/nbdkit-filter.h | 4 ++++ > include/nbdkit-plugin.h | 3 +++ > server/internal.h | 9 ++++++++ > server/backend.c | 29 +++++++++++++++++++++++ > server/filters.c | 17 ++++++++++++++ > server/plugins.c | 51 +++++++++++++++++++++++++++++++++++++++++ > 6 files changed, 113 insertions(+) >> > +int > +backend_block_size (struct context *c, > + uint32_t *minimum, uint32_t *preferred, uint32_t *maximum) > +{ > + PUSH_CONTEXT_FOR_SCOPE (c); > + struct backend *b = c->b; > + int r; > + > + assert (c->handle && (c->state & HANDLE_CONNECTED)); > + if (c->minimum_block_size != -1) { > + *minimum = c->minimum_block_size; > + *preferred = c->preferred_block_size; > + *maximum = c->maximum_block_size; > + return 0; > + } > + else { > + controlpath_debug ("%s: block_size", b->name); > + r = b->block_size (c, minimum, preferred, maximum); > + if (r == 0) { > + c->minimum_block_size = *minimum; > + c->preferred_block_size = *preferred; > + c->maximum_block_size = *maximum; > + }We should probably ensure that NBD protocol constraints are met rather than just assuming the plugin gave us sane values: minimum must be power of 2 between 1 and 64k; preferred must be power of 2 between max(minsize,512) and 32M; maximum must be either -1 or a multiple of minsize (but not necessarily a power of 2). /me reads on...> +++ b/server/plugins.c > > +static int > +plugin_block_size (struct context *c, > + uint32_t *minimum, uint32_t *preferred, uint32_t *maximum) > +{ > + struct backend *b = c->b; > + struct backend_plugin *p = container_of (b, struct backend_plugin, backend); > + int r; > + > + if (p->plugin.block_size) { > + r = p->plugin.block_size (c->handle, minimum, preferred, maximum); > + if (r == 0) { > + /* To make scripting easier, it's permitted to set > + * minimum = preferred = maximum = 0 and return 0. > + * That means "no information", and works the same > + * way as the else clause below. > + */ > + if (*minimum == 0 && *preferred == 0 && *maximum == 0) > + return 0; > + > + if (*minimum < 1) { > + nbdkit_error ("plugin must set minimum block size >= 1"); > + r = -1; > + }In other words, either all three values are 0 (no info), or all three values are non-zero, ruling out partial info. Makes sense. We could instead decide to provide defaults to let plugins provide partial info (such as if minsize is nonzero but preferred is 0, then set preferred to min(minsize, 4k), but I don't know if it would be worth the extra complication.> + if (! is_power_of_2 (*minimum)) { > + nbdkit_error ("plugin must set minimum block size to a power of 2"); > + r = -1; > + } > + if (*minimum > *preferred || *preferred > *maximum) { > + nbdkit_error ("plugin must set minimum block size " > + "<= preferred <= maximum"); > + r = -1;The NBD protocol allows the maximum to match the block size (and potentially be smaller than preferred size) if the overall export is small (for example, a 512-byte file can advertise a preferred size of 4k but a max size matching the actual size of 512). Yeah, it's a corner case, so I'm also okay if we insist that if a plugin give us information, the info be strictly ranked rather than allowing all corner cases possible in the NBD protocol.> + } > + /* XXX Other checks should be performed here: > + * preferred and maximum should be multiples of the minimum. > + */More specifically, preferred must also be a power of 2.> + } > + return r; > + } > + else { > + /* If there is no .block_size call then return minimum = preferred > + * = maximum = 0, which is a sentinel meaning don't send the > + * NBD_INFO_BLOCK_SIZE message. > + */ > + *minimum = *preferred = *maximum = 0;Yes, supporting a sentinel as the ability to turn off the message makes sense.> + return 0; > + } > +} > + > static int > normalize_bool (int value) > { > @@ -824,6 +874,7 @@ static struct backend plugin_functions = { > .close = plugin_close, > .export_description = plugin_export_description, > .get_size = plugin_get_size, > + .block_size = plugin_block_size, > .can_write = plugin_can_write, > .can_flush = plugin_can_flush, > .is_rotational = plugin_is_rotational, > -- > 2.35.1Looks like a good start. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org