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

If the client requests NBD_INFO_BLOCK_SIZE, _and_ either the plugin or a filter provides this information, then send a reply. Otherwise ignore the client request. --- server/protocol-handshake-newstyle.c | 69 ++++++++++++++++++++++++++-- TODO | 3 +- 2 files changed, 66 insertions(+), 6 deletions(-) diff --git a/server/protocol-handshake-newstyle.c b/server/protocol-handshake-newstyle.c index 5c073af6..f3cba48f 100644 --- a/server/protocol-handshake-newstyle.c +++ b/server/protocol-handshake-newstyle.c @@ -195,6 +195,38 @@ send_newstyle_option_reply_info_str (uint32_t option, uint32_t reply, return 0; } +/* Send reply containing NBD_INFO_BLOCK_SIZE. */ +static int +send_newstyle_option_reply_info_block_size (uint32_t option, uint32_t reply, + uint16_t info, + uint32_t minimum, + uint32_t preferred, + uint32_t maximum) +{ + GET_CONN; + struct nbd_fixed_new_option_reply fixed_new_option_reply; + struct nbd_fixed_new_option_reply_info_block_size block_size; + + fixed_new_option_reply.magic = htobe64 (NBD_REP_MAGIC); + fixed_new_option_reply.option = htobe32 (option); + fixed_new_option_reply.reply = htobe32 (reply); + fixed_new_option_reply.replylen = htobe32 (14); + block_size.info = htobe16 (info); + block_size.minimum = htobe32 (minimum); + block_size.preferred = htobe32 (preferred); + block_size.maximum = htobe32 (maximum); + + if (conn->send (&fixed_new_option_reply, + sizeof fixed_new_option_reply, SEND_MORE) == -1 || + conn->send (&block_size, + sizeof block_size, 0) == -1) { + nbdkit_error ("write: %s: %m", name_of_nbd_opt (option)); + return -1; + } + + return 0; +} + static int send_newstyle_option_reply_meta_context (uint32_t option, uint32_t reply, uint32_t context_id, @@ -589,10 +621,10 @@ negotiate_handshake_newstyle_options (void) exportsize) == -1) return -1; - /* For now we send NBD_INFO_NAME and NBD_INFO_DESCRIPTION if - * requested, and ignore all other info requests (including - * NBD_INFO_EXPORT if it was requested, because we replied - * already above). + /* For now we send NBD_INFO_NAME, NBD_INFO_DESCRIPTION and + * NBD_INFO_BLOCK_SIZE if requested, and ignore all other info + * requests (including NBD_INFO_EXPORT if it was requested, + * because we replied already above). */ for (i = 0; i < nrinfos; ++i) { memcpy (&info, &data[4 + exportnamelen + 2 + i*2], 2); @@ -637,6 +669,35 @@ negotiate_handshake_newstyle_options (void) return -1; } break; + case NBD_INFO_BLOCK_SIZE: + { + uint32_t minimum, preferred, maximum; + int r = backend_block_size (conn->top_context, + &minimum, &preferred, &maximum); + + if (r == -1) { + debug ("newstyle negotiation: %s: " + "NBD_INFO_BLOCK_SIZE: error from plugin, " + "ignoring client request", + optname); + break; + } + if (minimum == 0) { + debug ("newstyle negotiation: %s: " + "NBD_INFO_BLOCK_SIZE: client requested but " + "no plugin or filter provided block size information, " + "ignoring client request", + optname); + break; + } + if (send_newstyle_option_reply_info_block_size + (option, + NBD_REP_INFO, + NBD_INFO_BLOCK_SIZE, + minimum, preferred, maximum) == -1) + return -1; + } + break; default: debug ("newstyle negotiation: %s: " "ignoring NBD_INFO_* request %u (%s)", diff --git a/TODO b/TODO index a4a0e0e7..35e68d15 100644 --- a/TODO +++ b/TODO @@ -31,8 +31,7 @@ General ideas for improvements https://www.redhat.com/archives/libguestfs/2018-January/msg00149.html * More NBD protocol features. The currently missing features are - structured replies for sparse reads, block size constraints, and - online resize. + structured replies for sparse reads, and online resize. * Add a callback to let plugins request minimum alignment for the buffer to pread/pwrite; useful for a plugin utilizing O_DIRECT or -- 2.35.1

--- docs/nbdkit-filter.pod | 9 +++++++++ docs/nbdkit-plugin.pod | 28 ++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/docs/nbdkit-filter.pod b/docs/nbdkit-filter.pod index 36151e69..d63ee559 100644 --- a/docs/nbdkit-filter.pod +++ b/docs/nbdkit-filter.pod @@ -711,6 +711,15 @@ This intercepts the plugin C<.export_description> method and can be used to read or modify the export description that the NBD client will see. +=head2 C<.block_size> + + int block_size (nbdkit_next *next, void *handle, uint32_t *minimum, + uint32_t *preferred, uint32_t *maximum); + +This intercepts the plugin C<.block_size> method and can be used to +read or modify the block size constraints that the NBD client will +see. + =head2 C<.can_write> =head2 C<.can_flush> diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod index 329bcd31..15573f0a 100644 --- a/docs/nbdkit-plugin.pod +++ b/docs/nbdkit-plugin.pod @@ -872,6 +872,34 @@ returning a compile-time constant string, you may find C<nbdkit_strdup_intern> helpful for returning a value that avoids a memory leak. +=head2 C<.block_size> + + int block_size (void *handle, uint32_t *minimum, + uint32_t *preferred, uint32_t *maximum); + +This is called during the option negotiation phase of the protocol to +get the minimum, preferred and maximum block size (all in bytes) of +the block device. The client should obey these by constraints by not +making requests which are smaller than the minimum size or larger than +the maximum size, and usually making requests of the preferred size. +Furthermore requests should be aligned to at least the minimum block +size, and usually the preferred block size. + +Even if the plugin implements this callback, this does B<not> mean +that all client requests will obey the constraints. A client could +still ignore the constraints. nbdkit passes all requests through to +the plugin, because what the plugin does depends on the plugin's +policy. It might decide to serve the requests correctly anyway, or +reject them with an error. + +The minimum block size must be E<ge> 1. The maximum block size must +be E<le> 0xffff_ffff. minimum E<le> preferred E<le> maximum. + +If this callback is not used, then the NBD protocol assumes by default +minimum = 1, preferred = 4096. (Maximum block size depends on various +factors, see the NBD protocol specification, section "Block size +constraints"). + =head2 C<.can_write> int can_write (void *handle); -- 2.35.1

The main reason to implement block_size in these plugins first is so we then have an easy way to implement tests of the feature. --- plugins/eval/nbdkit-eval-plugin.pod | 2 + plugins/sh/nbdkit-sh-plugin.pod | 10 +++++ plugins/sh/methods.h | 3 ++ plugins/eval/eval.c | 2 + plugins/sh/methods.c | 66 +++++++++++++++++++++++++++++ plugins/sh/sh.c | 1 + 6 files changed, 84 insertions(+) diff --git a/plugins/eval/nbdkit-eval-plugin.pod b/plugins/eval/nbdkit-eval-plugin.pod index 22167ec5..fa9784ae 100644 --- a/plugins/eval/nbdkit-eval-plugin.pod +++ b/plugins/eval/nbdkit-eval-plugin.pod @@ -70,6 +70,8 @@ features): =item B<after_fork=>SCRIPT +=item B<block_size=>SCRIPT + =item B<cache=>SCRIPT =item B<can_cache=>SCRIPT diff --git a/plugins/sh/nbdkit-sh-plugin.pod b/plugins/sh/nbdkit-sh-plugin.pod index c4cb9fac..aede1c65 100644 --- a/plugins/sh/nbdkit-sh-plugin.pod +++ b/plugins/sh/nbdkit-sh-plugin.pod @@ -372,6 +372,16 @@ L<nbdkit-plugin(3)/PARSING SIZE PARAMETERS>). This method is required. +=item C<block_size> + + /path/to/script block_size <handle> + +This script should print three numbers on stdout, separated by +whitespace. These are (in order) the minimum block size, the +preferred block size, and the maximum block size. You can print the +sizes in bytes or use any format understood by C<nbdkit_parse_size> +such as C<1M> (see L<nbdkit-plugin(3)/PARSING SIZE PARAMETERS>). + =item C<can_write> =item C<can_flush> diff --git a/plugins/sh/methods.h b/plugins/sh/methods.h index 42eb560c..d405ef00 100644 --- a/plugins/sh/methods.h +++ b/plugins/sh/methods.h @@ -51,6 +51,9 @@ extern void *sh_open (int readonly); extern void sh_close (void *handle); extern const char *sh_export_description (void *handle); extern int64_t sh_get_size (void *handle); +extern int sh_block_size (void *handle, + uint32_t *minimum, uint32_t *preferred, + uint32_t *maximum); extern int sh_pread (void *handle, void *buf, uint32_t count, uint64_t offset, uint32_t flags); extern int sh_pwrite (void *handle, const void *buf, uint32_t count, diff --git a/plugins/eval/eval.c b/plugins/eval/eval.c index b312a59c..8ce7a650 100644 --- a/plugins/eval/eval.c +++ b/plugins/eval/eval.c @@ -55,6 +55,7 @@ static char *missing; static const char *known_methods[] = { "after_fork", + "block_size", "cache", "can_cache", "can_extents", @@ -402,6 +403,7 @@ static struct nbdkit_plugin plugin = { .export_description = sh_export_description, .get_size = sh_get_size, + .block_size = sh_block_size, .can_write = sh_can_write, .can_flush = sh_can_flush, .is_rotational = sh_is_rotational, diff --git a/plugins/sh/methods.c b/plugins/sh/methods.c index 51bc89e5..1216c212 100644 --- a/plugins/sh/methods.c +++ b/plugins/sh/methods.c @@ -526,6 +526,72 @@ sh_get_size (void *handle) } } +int +sh_block_size (void *handle, + uint32_t *minimum, uint32_t *preferred, uint32_t *maximum) +{ + const char *method = "block_size"; + const char *script = get_script (method); + struct sh_handle *h = handle; + const char *args[] = { script, method, h->h, NULL }; + CLEANUP_FREE char *s = NULL; + size_t slen; + const char *delim = " \t\n"; + char *sp, *p; + int64_t r; + + switch (call_read (&s, &slen, args)) { + case OK: + if ((p = strtok_r (s, delim, &sp)) == NULL) { + parse_error: + nbdkit_error ("%s: %s method cannot be parsed", script, method); + return -1; + } + r = nbdkit_parse_size (p); + if (r == -1 || r > UINT32_MAX) + goto parse_error; + *minimum = r; + + if ((p = strtok_r (NULL, delim, &sp)) == NULL) + goto parse_error; + r = nbdkit_parse_size (p); + if (r == -1 || r > UINT32_MAX) + goto parse_error; + *preferred = r; + + if ((p = strtok_r (NULL, delim, &sp)) == NULL) + goto parse_error; + r = nbdkit_parse_size (p); + if (r == -1 || r > UINT32_MAX) + goto parse_error; + *maximum = r; + +#if 0 + nbdkit_debug ("setting block_size: " + "minimum=%" PRIu32 " " + "preferred=%" PRIu32 " " + "maximum=%" PRIu32, + *minimum, *preferred, *maximum); +#endif + return 0; + + case MISSING: + *minimum = *preferred = *maximum = 0; + return 0; + + case ERROR: + return -1; + + case RET_FALSE: + nbdkit_error ("%s: %s method returned unexpected code (3/false)", + script, method); + errno = EIO; + return -1; + + default: abort (); + } +} + int sh_pread (void *handle, void *buf, uint32_t count, uint64_t offset, uint32_t flags) diff --git a/plugins/sh/sh.c b/plugins/sh/sh.c index db75d386..35972104 100644 --- a/plugins/sh/sh.c +++ b/plugins/sh/sh.c @@ -307,6 +307,7 @@ static struct nbdkit_plugin plugin = { .export_description = sh_export_description, .get_size = sh_get_size, + .block_size = sh_block_size, .can_write = sh_can_write, .can_flush = sh_can_flush, .is_rotational = sh_is_rotational, -- 2.35.1

--- tests/Makefile.am | 4 +++ tests/test-block-size-constraints.sh | 50 ++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/tests/Makefile.am b/tests/Makefile.am index 16f76eb8..e888b1a0 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -562,6 +562,10 @@ EXTRA_DIST += test-eflags.sh TESTS += test-export-name.sh test-export-info.sh EXTRA_DIST += test-export-name.sh test-export-info.sh +# Test block size constraints. +TESTS += test-block-size-constraints.sh +EXTRA_DIST += test-block-size-constraints.sh + # cdi plugin test. TESTS += test-cdi.sh EXTRA_DIST += test-cdi.sh diff --git a/tests/test-block-size-constraints.sh b/tests/test-block-size-constraints.sh new file mode 100755 index 00000000..658445cd --- /dev/null +++ b/tests/test-block-size-constraints.sh @@ -0,0 +1,50 @@ +#!/usr/bin/env bash +# nbdkit +# Copyright (C) 2019-2022 Red Hat Inc. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# +# * Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution. +# +# * Neither the name of Red Hat nor the names of its contributors may be +# used to endorse or promote products derived from this software without +# specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS IS'' AND +# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, +# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A +# PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR +# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF +# USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND +# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT +# OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF +# SUCH DAMAGE. + +source ./functions.sh +set -e +set -x + +requires_plugin eval +requires nbdsh -c 'print(h.get_block_size)' + +# Create an nbdkit eval plugin which presents block size constraints. +# Check the advertised block size constraints can be read. +nbdkit -U - eval \ + block_size="echo 64K 128K 32M" \ + get_size="echo 0" \ + --run 'nbdsh \ + -u "$uri" \ + -c "assert h.get_block_size(nbd.SIZE_MINIMUM) == 64 * 1024" \ + -c "assert h.get_block_size(nbd.SIZE_PREFERRED) == 128 * 1024" \ + -c "assert h.get_block_size(nbd.SIZE_MAXIMUM) == 32 * 1024 * 1024" \ + ' -- 2.35.1

This filter can add block size constraints to plugins which don't already support them. It can also enforce an error policy for badly behaved clients which do not obey the block size constraints. UNFINISHED: The error policy is not implemented. --- docs/nbdkit-plugin.pod | 4 +- .../nbdkit-block-size-constraint-filter.pod | 122 +++++++++++ configure.ac | 2 + filters/block-size-constraint/Makefile.am | 70 ++++++ tests/Makefile.am | 4 + .../block-size-constraint.c | 206 ++++++++++++++++++ tests/test-block-size-constraint-filter.sh | 117 ++++++++++ 7 files changed, 524 insertions(+), 1 deletion(-) diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod index 15573f0a..427ca428 100644 --- a/docs/nbdkit-plugin.pod +++ b/docs/nbdkit-plugin.pod @@ -890,7 +890,9 @@ that all client requests will obey the constraints. A client could still ignore the constraints. nbdkit passes all requests through to the plugin, because what the plugin does depends on the plugin's policy. It might decide to serve the requests correctly anyway, or -reject them with an error. +reject them with an error. Plugins can avoid this complexity by using +L<nbdkit-block-size-constraint-filter(1)> which allows both +setting/adjusting the constraints, and selecting an error policy. The minimum block size must be E<ge> 1. The maximum block size must be E<le> 0xffff_ffff. minimum E<le> preferred E<le> maximum. diff --git a/filters/block-size-constraint/nbdkit-block-size-constraint-filter.pod b/filters/block-size-constraint/nbdkit-block-size-constraint-filter.pod new file mode 100644 index 00000000..9205e503 --- /dev/null +++ b/filters/block-size-constraint/nbdkit-block-size-constraint-filter.pod @@ -0,0 +1,122 @@ +=head1 NAME + +nbdkit-block-size-constraint-filter - set minimum, preferred and +maximum block size, and apply error policy + +=head1 SYNOPSIS + + nbdkit --filter=block-size-constraint PLUGIN + [block-size-error-policy=allow|error] + [block-size-minimum=N] + [block-size-preferred=N] + [block-size-maximum=N] + +=head1 DESCRIPTION + +C<nbdkit-block-size-constraint-filter> is an L<nbdkit(1)> filter that +can add block size constraints to plugins which don't already support +then. It can also enforce an error policy for badly behaved clients +which do not obey the block size constraints. + +For more information about block size constraints, see section +"Block size constraints" in +L<https://github.com/NetworkBlockDevice/nbd/blob/master/doc/proto.md>. + +The simplest usage is to place this filter on top of any plugin which +does not advertise block size constraints, and set the +C<block-size-minimum>, C<block-size-preferred> and +C<block-size-maximum> parameters with the desired constraints. For +example: + + nbdkit --filter=block-size-constraint memory 1G \ + block-size-preferred=32K + +would adjust L<nbdkit-memory-plugin(1)> so that clients should +prefer 32K requests. You can query the NBD server advertised constraints +using L<nbdinfo(1)>: + + $ nbdinfo nbd://localhost + [...] + block_size_minimum: 1 + block_size_preferred: 32768 + block_size_maximum: 4294967295 + +The second part of this filter is adjusting the error policy when +badly behaved clients do not obey the minimum or maximum request size. +Normally nbdkit permits these requests, leaving it up to the plugin +whether it rejects the request with an error or tries to process the +request (eg. trying to split an over-large request). With this filter +you can use C<block-size-error-policy=error> to reject these requests +in the filter with EIO error. The plugin will not see them. + +A related filter is L<nbdkit-blocksize-filter(1)>. That filter can +split and combine requests for plugins that cannot handle requests +under or over a particular size. The two filters may be used together +- this filter advertising what the plugin can support to well-behaved +clients, and the other filter coping with clients that are either not +well-behaved or not capable of adjusting request size. + +=head1 PARAMETERS + +=over 4 + +=item B<block-size-error-policy=allow> + +=item B<block-size-error-policy=error> + +If a client sends a request which is smaller than the permitted +minimum size or larger than the permitted maximum size, +C<block-size-error-policy> chooses what the filter will do. The +default (and also nbdkit's default) is C<allow> which means pass the +request through to the plugin. Use C<error> to return an EIO error +back to the client. The plugin will not see the badly formed request +in this case. + +=item B<block-size-minimum=>N + +=item B<block-size-preferred=>N + +=item B<block-size-maximum=>N + +Advertise minimum, preferred and/or maximum block size to the client. +Well-behaved clients should obey these constraints. + +For each parameter, you can specify it as a size (using the usual +modifiers like C<4K>). + +If the parameter is omitted then either the constraint advertised by +the plugin itself is used, or a sensible default for plugins which do +not advertise block size constraints. + +=back + +=head1 FILES + +=over 4 + +=item F<$filterdir/nbdkit-block-size-constraint-filter.so> + +The filter. + +Use C<nbdkit --dump-config> to find the location of C<$filterdir>. + +=back + +=head1 VERSION + +C<nbdkit-limit-filter> first appeared in nbdkit 1.30. + +=head1 SEE ALSO + +L<nbdkit(1)>, +L<nbdkit-blocksize-filter(1)>, +L<nbdkit-filter(3)>, +L<nbdkit-plugin(3)>. + +=head1 AUTHORS + +Richard W.M. Jones + +=head1 COPYRIGHT + +Copyright (C) 2022 Red Hat Inc. diff --git a/configure.ac b/configure.ac index 1c7200c9..adccd90e 100644 --- a/configure.ac +++ b/configure.ac @@ -109,6 +109,7 @@ non_lang_plugins="\ plugins="$(echo $(printf %s\\n $lang_plugins $non_lang_plugins | sort -u))" filters="\ blocksize \ + block-size-constraint \ cache \ cacheextents \ checkwrite \ @@ -1335,6 +1336,7 @@ AC_CONFIG_FILES([Makefile plugins/zero/Makefile filters/Makefile filters/blocksize/Makefile + filters/block-size-constraint/Makefile filters/cache/Makefile filters/cacheextents/Makefile filters/checkwrite/Makefile diff --git a/filters/block-size-constraint/Makefile.am b/filters/block-size-constraint/Makefile.am new file mode 100644 index 00000000..244f8d66 --- /dev/null +++ b/filters/block-size-constraint/Makefile.am @@ -0,0 +1,70 @@ +# nbdkit +# Copyright (C) 2019-2021 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-block-size-constraint-filter.pod + +filter_LTLIBRARIES = nbdkit-block-size-constraint-filter.la + +nbdkit_block_size_constraint_filter_la_SOURCES = \ + block-size-constraint.c \ + $(top_srcdir)/include/nbdkit-filter.h \ + $(NULL) + +nbdkit_block_size_constraint_filter_la_CPPFLAGS = \ + -I$(top_srcdir)/common/include \ + -I$(top_srcdir)/common/utils \ + -I$(top_srcdir)/include \ + $(NULL) +nbdkit_block_size_constraint_filter_la_CFLAGS = $(WARNINGS_CFLAGS) +nbdkit_block_size_constraint_filter_la_LDFLAGS = \ + -module -avoid-version -shared $(NO_UNDEFINED_ON_WINDOWS) \ + -Wl,--version-script=$(top_srcdir)/filters/filters.syms \ + $(NULL) +nbdkit_block_size_constraint_filter_la_LIBADD = \ + $(top_builddir)/common/utils/libutils.la \ + $(top_builddir)/common/replacements/libcompat.la \ + $(IMPORT_LIBRARY_ON_WINDOWS) \ + $(NULL) + +if HAVE_POD + +man_MANS = nbdkit-block-size-constraint-filter.1 +CLEANFILES += $(man_MANS) + +nbdkit-block-size-constraint-filter.1: nbdkit-block-size-constraint-filter.pod \ + $(top_builddir)/podwrapper.pl + $(PODWRAPPER) --section=1 --man $@ \ + --html $(top_builddir)/html/$@.html \ + $< + +endif HAVE_POD diff --git a/tests/Makefile.am b/tests/Makefile.am index e888b1a0..042cd620 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -1409,6 +1409,10 @@ test_layers_filter3_la_LIBADD = $(IMPORT_LIBRARY_ON_WINDOWS) TESTS += test-blocksize.sh test-blocksize-extents.sh EXTRA_DIST += test-blocksize.sh test-blocksize-extents.sh +# block-size-constraint filter test. +TESTS += test-block-size-constraint-filter.sh +EXTRA_DIST += test-block-size-constraint-filter.sh + # cache filter test. TESTS += \ test-cache.sh \ diff --git a/filters/block-size-constraint/block-size-constraint.c b/filters/block-size-constraint/block-size-constraint.c new file mode 100644 index 00000000..2b4e2e30 --- /dev/null +++ b/filters/block-size-constraint/block-size-constraint.c @@ -0,0 +1,206 @@ +/* nbdkit + * Copyright (C) 2019-2022 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 <stdint.h> +#include <inttypes.h> +#include <string.h> +#include <pthread.h> + +#include <nbdkit-filter.h> + +#include "ispowerof2.h" + +/* Block size constraints configured on the command line (0 = unset). */ +static uint32_t config_minimum; +static uint32_t config_preferred; +static uint32_t config_maximum; + +/* Error policy. */ +static enum { ALLOW, ERROR } error_policy = ALLOW; + +static int +block_size_constraint_config (nbdkit_next_config *next, nbdkit_backend *nxdata, + const char *key, const char *value) +{ + int64_t r; + + if (strcmp (key, "block-size-error-policy") == 0) { + if (strcmp (value, "allow") == 0) + error_policy = ALLOW; + else if (strcmp (value, "error") == 0) + error_policy = ERROR; + else { + nbdkit_error ("unknown %s: %s", key, value); + return -1; + } + return 0; + } + else if (strcmp (key, "block-size-minimum") == 0) { + r = nbdkit_parse_size (value); + if (r == -1 || r > UINT32_MAX) { + parse_error: + nbdkit_error ("%s: could not parse %s", key, value); + return -1; + } + config_minimum = r; + return 0; + } + else if (strcmp (key, "block-size-preferred") == 0) { + r = nbdkit_parse_size (value); + if (r == -1 || r > UINT32_MAX) goto parse_error; + config_preferred = r; + return 0; + } + else if (strcmp (key, "block-size-maximum") == 0) { + r = nbdkit_parse_size (value); + if (r == -1 || r > UINT32_MAX) goto parse_error; + config_maximum = r; + return 0; + } + + return next (nxdata, key, value); +} + +static int +block_size_constraint_config_complete (nbdkit_next_config_complete *next, + nbdkit_backend *nxdata) +{ + /* Just do some basic checks that the values are sane (if set). */ + if (config_minimum) { + if (! is_power_of_2 (config_minimum)) { + nbdkit_error ("block-size-minimum must be a power of 2"); + return -1; + } + } + + if (config_minimum && config_preferred) { + if (config_minimum > config_preferred) { + nbdkit_error ("block-size-minimum must be <= block-size-preferred"); + return -1; + } + } + + if (config_preferred && config_maximum) { + if (config_preferred > config_maximum) { + nbdkit_error ("block-size-preferred must be <= block-size-maximum"); + return -1; + } + } + + /* XXX We could also check the preferred and maximum are a + * multiple of minimum. + */ + + return next (nxdata); +} + +static int +block_size_constraint_block_size (nbdkit_next *next, + void *handle, + uint32_t *minimum, uint32_t *preferred, + uint32_t *maximum) +{ + /* If the user has set all of the block size parameters then we + * don't need to ask the plugin, we can go ahead and advertise them. + */ + if (config_minimum && config_preferred && config_maximum) { + *minimum = config_minimum; + *preferred = config_preferred; + *maximum = config_maximum; + return 0; + } + + /* Otherwise, ask the plugin. */ + if (next->block_size (next, minimum, preferred, maximum) == -1) + return -1; + + /* If the user of this filter didn't configure anything, then return + * the plugin values (even if unset). + */ + if (!config_minimum && !config_preferred && !config_maximum) + return 0; + + /* Now we get to the awkward case where the user configured some + * values but not others. There's all kinds of room for things to + * go wrong here, so try to check for obvious user errors as best we + * can. + */ + if (*minimum == 0) { /* Plugin didn't set anything. */ + if (config_minimum) + *minimum = config_minimum; + else + *minimum = 1; + + if (config_preferred) + *preferred = config_preferred; + else + *preferred = 4096; + + if (config_maximum) + *maximum = config_maximum; + else + *maximum = 0xffffffff; + } + else { /* Plugin set some values. */ + if (config_minimum) + *minimum = config_minimum; + + if (config_preferred) + *preferred = config_preferred; + + if (config_maximum) + *maximum = config_maximum; + } + + if (*minimum > *preferred || *preferred > *maximum) { + nbdkit_error ("computed block size values are invalid, minimum %" PRIu32 + " > preferred %" PRIu32 + " or preferred > maximum %" PRIu32, + *minimum, *preferred, *maximum); + return -1; + } + return 0; +} + +static struct nbdkit_filter filter = { + .name = "block-size-constraint", + .longname = "nbdkit block size constraint filter", + .config = block_size_constraint_config, + .config_complete = block_size_constraint_config_complete, + .block_size = block_size_constraint_block_size, +}; + +NBDKIT_REGISTER_FILTER(filter) diff --git a/tests/test-block-size-constraint-filter.sh b/tests/test-block-size-constraint-filter.sh new file mode 100755 index 00000000..5c240e73 --- /dev/null +++ b/tests/test-block-size-constraint-filter.sh @@ -0,0 +1,117 @@ +#!/usr/bin/env bash +# nbdkit +# Copyright (C) 2019-2022 Red Hat Inc. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# +# * Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution. +# +# * Neither the name of Red Hat nor the names of its contributors may be +# used to endorse or promote products derived from this software without +# specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS IS'' AND +# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, +# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A +# PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR +# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF +# USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND +# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT +# OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF +# SUCH DAMAGE. + +source ./functions.sh +set -e +set -x + +requires_plugin eval +requires nbdsh -c 'print(h.get_block_size)' + +# Run nbdkit eval + filter and check resulting block size constraints +# using some nbdsh. + +# No parameters. +nbdkit -U - eval \ + block_size="echo 64K 128K 32M" \ + get_size="echo 0" \ + --filter=block-size-constraint \ + --run 'nbdsh \ + -u "$uri" \ + -c "assert h.get_block_size(nbd.SIZE_MINIMUM) == 64 * 1024" \ + -c "assert h.get_block_size(nbd.SIZE_PREFERRED) == 128 * 1024" \ + -c "assert h.get_block_size(nbd.SIZE_MAXIMUM) == 32 * 1024 * 1024" \ + ' + +# Adjust single values. +nbdkit -U - eval \ + block_size="echo 64K 128K 32M" \ + get_size="echo 0" \ + --filter=block-size-constraint \ + block-size-minimum=1 \ + --run 'nbdsh \ + -u "$uri" \ + -c "assert h.get_block_size(nbd.SIZE_MINIMUM) == 1" \ + -c "assert h.get_block_size(nbd.SIZE_PREFERRED) == 128 * 1024" \ + -c "assert h.get_block_size(nbd.SIZE_MAXIMUM) == 32 * 1024 * 1024" \ + ' +nbdkit -U - eval \ + block_size="echo 64K 128K 32M" \ + get_size="echo 0" \ + --filter=block-size-constraint \ + block-size-preferred=64K \ + --run 'nbdsh \ + -u "$uri" \ + -c "assert h.get_block_size(nbd.SIZE_MINIMUM) == 64 * 1024" \ + -c "assert h.get_block_size(nbd.SIZE_PREFERRED) == 64 * 1024" \ + -c "assert h.get_block_size(nbd.SIZE_MAXIMUM) == 32 * 1024 * 1024" \ + ' +nbdkit -U - eval \ + block_size="echo 64K 128K 32M" \ + get_size="echo 0" \ + --filter=block-size-constraint \ + block-size-maximum=1M \ + --run 'nbdsh \ + -u "$uri" \ + -c "assert h.get_block_size(nbd.SIZE_MINIMUM) == 64 * 1024" \ + -c "assert h.get_block_size(nbd.SIZE_PREFERRED) == 128 * 1024" \ + -c "assert h.get_block_size(nbd.SIZE_MAXIMUM) == 1 * 1024 * 1024" \ + ' + +# Adjust all values for a plugin which is advertising. +nbdkit -U - eval \ + block_size="echo 64K 128K 32M" \ + get_size="echo 0" \ + --filter=block-size-constraint \ + block-size-minimum=1 \ + block-size-preferred=4K \ + block-size-maximum=1M \ + --run 'nbdsh \ + -u "$uri" \ + -c "assert h.get_block_size(nbd.SIZE_MINIMUM) == 1" \ + -c "assert h.get_block_size(nbd.SIZE_PREFERRED) == 4 * 1024" \ + -c "assert h.get_block_size(nbd.SIZE_MAXIMUM) == 1 * 1024 * 1024" \ + ' + +# Set all values for a plugin which is not advertising. +nbdkit -U - eval \ + get_size="echo 0" \ + --filter=block-size-constraint \ + block-size-minimum=1 \ + block-size-preferred=4K \ + block-size-maximum=1M \ + --run 'nbdsh \ + -u "$uri" \ + -c "assert h.get_block_size(nbd.SIZE_MINIMUM) == 1" \ + -c "assert h.get_block_size(nbd.SIZE_PREFERRED) == 4 * 1024" \ + -c "assert h.get_block_size(nbd.SIZE_MAXIMUM) == 1 * 1024 * 1024" \ + ' -- 2.35.1