[Libguestfs] [libnbd PATCH v2 3/5] api: Add nbd_set_strict_mode

Right now, libnbd has refused to issue a command not advertised as supported by a server, mainly because the NBD protocol does not guarantee what the server will do, and libnbd would rather stay in sync with the server than drop the connection. However, for integration purposes, it can be handy to coerce libnbd into sending something to see how the server will react (whether it be an extension libnbd has not yet learned, or an intentional bad request to test server error handling). Time to make this something the user can control, by adding a new strictness mode. Later patches will add more knobs to the mode. --- lib/internal.h | 3 + generator/API.ml | 165 ++++++++++++++++++++++++++++++++++++----------- lib/handle.c | 16 +++++ lib/rw.c | 134 +++++++++++++++++++++----------------- tests/errors.c | 21 +++++- 5 files changed, 240 insertions(+), 99 deletions(-) diff --git a/lib/internal.h b/lib/internal.h index 96699b5..2a5147f 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -148,6 +148,9 @@ struct nbd_handle { bool debug; nbd_debug_callback debug_callback; + /* How strict to be. */ + uint32_t strict; + /* State machine. * * The actual current state is ‘state’. ‘public_state’ is updated diff --git a/generator/API.ml b/generator/API.ml index b212e95..aa970e6 100644 --- a/generator/API.ml +++ b/generator/API.ml @@ -102,6 +102,13 @@ and link = | ExternalLink of string * int | URLLink of string +let strict_call_description = "\n +By default, libnbd will reject attempts to use this function with +parameters that are likely to result in server failure, such as +requesting an unknown command flag. The L<nbd_set_strict_mode(3)> +function can be used to alter which scenarios should await a server +reply rather than failing fast." + let non_blocking_test_call_description = "\n This call does not block, because it returns data that is saved in the handle from the NBD protocol handshake." @@ -171,7 +178,13 @@ let handshake_flags = { flags = [ "FIXED_NEWSTYLE", 1 lsl 0; "NO_ZEROES", 1 lsl 1; - ] + ] +} +let strict_flags = { + flag_prefix = "STRICT"; + flags = [ + "COMMANDS", 1 lsl 0; + ] } let allow_transport_flags = { flag_prefix = "ALLOW_TRANSPORT"; @@ -187,8 +200,8 @@ let shutdown_flags = { "IMMEDIATE", 1 lsl 1; ] } -let all_flags = [ cmd_flags; handshake_flags; allow_transport_flags; - shutdown_flags ] +let all_flags = [ cmd_flags; handshake_flags; strict_flags; + allow_transport_flags; shutdown_flags ] let default_call = { args = []; optargs = []; ret = RErr; shortdesc = ""; longdesc = ""; example = None; @@ -451,7 +464,7 @@ test whether this is the case with L<nbd_supports_tls(3)>."; "get_tls", { default_call with - args = []; ret = REnum (tls_enum); + args = []; ret = REnum tls_enum; may_set_error = false; shortdesc = "get the TLS request setting"; longdesc = "\ @@ -610,7 +623,7 @@ for integration testing, it can be useful to clear this flag rather than find a way to alter the server to fail the negotiation request."; see_also = [Link "get_request_structured_replies"; - Link "set_handshake_flags"; + Link "set_handshake_flags"; Link "set_strict_mode"; Link "get_structured_replies_negotiated"; Link "can_meta_context"; Link "can_df"]; }; @@ -692,7 +705,7 @@ blindly setting a constant value."; "get_handshake_flags", { default_call with - args = []; ret = RFlags (handshake_flags); + args = []; ret = RFlags handshake_flags; may_set_error = false; shortdesc = "see which handshake flags are supported"; longdesc = "\ @@ -706,10 +719,62 @@ protocol defines new handshake flags, then the return value from a newer library version may include bits that were undefined at the time of compilation."; see_also = [Link "set_handshake_flags"; - Link "get_protocol"; + Link "get_protocol"; Link "set_strict_mode"; Link "aio_is_created"; Link "aio_is_ready"]; }; + "set_strict_mode", { + default_call with + args = [ Flags ("flags", strict_flags) ]; ret = RErr; + shortdesc = "control how strictly to follow NBD protocol"; + longdesc = "\ +By default, libnbd tries to detect requests that would trigger +undefined behavior in the NBD protocol, and rejects them client +side without causing any network traffic, rather than risking +undefined server behavior. However, for integration testing, it +can be handy to relax the strictness of libnbd, to coerce it into +sending such requests over the network for testing the robustness +of the server in dealing with such traffic. + +The C<flags> argument is a bitmask, including zero or more of the +following strictness flags: + +=over 4 + +=item C<LIBNBD_STRICT_COMMANDS> = 1 + +If set, this flag rejects client requests that do not comply with the +set of advertised server flags (for example, attempting a write on +a read-only server, or attempting to use C<LIBNBD_CMD_FLAG_FUA> when +L<nbd_can_fua(3)> returned false). If clear, this flag relies on the +server to reject unexpected commands. + +=back + +For convenience, the constant C<LIBNBD_STRICT_MASK> is available to +describe all strictness flags supported by this build of libnbd. +Future versions of libnbd may add further flags, which are likely +to be enabled by default for additional client-side filtering. As +such, when attempting to relax only one specific bit while keeping +remaining checks at the client side, it is wiser to first call +L<nbd_get_strict_mode(3)> and modify that value, rather than +blindly setting a constant value."; + see_also = [Link "get_strict_mode"; Link "set_handshake_flags"]; + }; + + "get_strict_mode", { + default_call with + args = []; ret = RFlags strict_flags; + may_set_error = false; + shortdesc = "see which strictness flags are in effect"; + longdesc = "\ +Return flags indicating which protocol strictness items are being +enforced locally by libnbd rather than the server. The return value +from a newer library version may include bits that were undefined at +the time of compilation."; + see_also = [Link "set_strict_mode"]; + }; + "set_opt_mode", { default_call with args = [Bool "enable"]; ret = RErr; @@ -1598,15 +1663,18 @@ Issue a write command to the NBD server, writing the data in C<buf> to the range starting at C<offset> and ending at C<offset> + C<count> - 1. NBD can only write all or nothing using this call. The call returns when the command has been -acknowledged by the server, or there is an error. +acknowledged by the server, or there is an error. Note this will +generally return an error if L<nbd_is_read_only(3)> is true. The C<flags> parameter may be C<0> for no flags, or may contain C<LIBNBD_CMD_FLAG_FUA> meaning that the server should not return until the data has been committed to permanent storage (if that is supported - some servers cannot do this, see -L<nbd_can_fua(3)>)."; +L<nbd_can_fua(3)>)." +^ strict_call_description; see_also = [Link "can_fua"; Link "is_read_only"; - Link "aio_pwrite"; Link "get_block_size"]; + Link "aio_pwrite"; Link "get_block_size"; + Link "set_strict_mode"]; example = Some "examples/reads-and-writes.c"; }; @@ -1657,11 +1725,13 @@ A future version of the library may add new flags."; Issue the flush command to the NBD server. The function should return when all write commands which have completed have been committed to permanent storage on the server. Note this will -return an error if L<nbd_can_flush(3)> is false. +generally return an error if L<nbd_can_flush(3)> is false. The C<flags> parameter must be C<0> for now (it exists for future NBD -protocol extensions)."; - see_also = [Link "can_flush"; Link "aio_flush"]; +protocol extensions)." +^ strict_call_description; + see_also = [Link "can_flush"; Link "aio_flush"; + Link "set_strict_mode"]; }; "trim", { @@ -1676,15 +1746,17 @@ Issue a trim command to the NBD server, which if supported by the server causes a hole to be punched in the backing store starting at C<offset> and ending at C<offset> + C<count> - 1. The call returns when the command has been acknowledged by the server, -or there is an error. +or there is an error. Note this will generally return an error +if L<nbd_can_trim(3)> is false or L<nbd_is_read_only(3)> is true. The C<flags> parameter may be C<0> for no flags, or may contain C<LIBNBD_CMD_FLAG_FUA> meaning that the server should not return until the data has been committed to permanent storage (if that is supported - some servers cannot do this, see -L<nbd_can_fua(3)>)."; - see_also = [Link "can_fua"; Link "can_trim"; - Link "aio_trim"]; +L<nbd_can_fua(3)>)." +^ strict_call_description; + see_also = [Link "can_fua"; Link "can_trim"; Link "is_read_only"; + Link "aio_trim"; Link "set_strict_mode"]; }; "cache", { @@ -1699,12 +1771,14 @@ Issue the cache (prefetch) command to the NBD server, which if supported by the server causes data to be prefetched into faster storage by the server, speeding up a subsequent L<nbd_pread(3)> call. The server can also silently ignore -this command. Note this will return an error if +this command. Note this will generally return an error if L<nbd_can_cache(3)> is false. The C<flags> parameter must be C<0> for now (it exists for future NBD -protocol extensions)."; - see_also = [Link "can_cache"; Link "aio_cache"]; +protocol extensions)." +^ strict_call_description; + see_also = [Link "can_cache"; Link "aio_cache"; + Link "set_strict_mode"]; }; "zero", { @@ -1720,7 +1794,8 @@ Issue a write zeroes command to the NBD server, which if supported by the server causes a zeroes to be written efficiently starting at C<offset> and ending at C<offset> + C<count> - 1. The call returns when the command has been acknowledged by the server, -or there is an error. +or there is an error. Note this will generally return an error if +L<nbd_can_zero(3)> is false or L<nbd_is_read_only(3)> is true. The C<flags> parameter may be C<0> for no flags, or may contain C<LIBNBD_CMD_FLAG_FUA> meaning that the server should not @@ -1731,9 +1806,11 @@ the server should favor writing actual allocated zeroes over punching a hole, and/or C<LIBNBD_CMD_FLAG_FAST_ZERO> meaning that the server must fail quickly if writing zeroes is no faster than a normal write (if that is supported - some servers -cannot do this, see L<nbd_can_fast_zero(3)>)."; - see_also = [Link "can_fua"; Link "can_zero"; - Link "can_fast_zero"; Link "aio_zero"]; +cannot do this, see L<nbd_can_fast_zero(3)>)." +^ strict_call_description; + see_also = [Link "can_fua"; Link "can_zero"; Link "is_read_only"; + Link "can_fast_zero"; Link "aio_zero"; + Link "set_strict_mode"]; }; "block_status", { @@ -1792,9 +1869,10 @@ The C<flags> parameter may be C<0> for no flags, or may contain C<LIBNBD_CMD_FLAG_REQ_ONE> meaning that the server should return only one extent per metadata context where that extent does not exceed C<count> bytes; however, libnbd does not -validate that the server obeyed the flag."; +validate that the server obeyed the flag." +^ strict_call_description; see_also = [Link "add_meta_context"; Link "can_meta_context"; - Link "aio_block_status"]; + Link "aio_block_status"; Link "set_strict_mode"]; }; "poll", { @@ -2088,9 +2166,10 @@ Or supply the optional C<completion_callback> which will be invoked as described in L<libnbd(3)/Completion callbacks>. Note that you must ensure C<buf> is valid until the command has -completed. Other parameters behave as documented in L<nbd_pwrite(3)>."; +completed. Other parameters behave as documented in L<nbd_pwrite(3)>." +^ strict_call_description; see_also = [SectionLink "Issuing asynchronous commands"; - Link "is_read_only"; Link "pwrite"]; + Link "is_read_only"; Link "pwrite"; Link "set_strict_mode"]; }; "aio_disconnect", { @@ -2132,9 +2211,10 @@ To check if the command completed, call L<nbd_aio_command_completed(3)>. Or supply the optional C<completion_callback> which will be invoked as described in L<libnbd(3)/Completion callbacks>. -Other parameters behave as documented in L<nbd_flush(3)>."; +Other parameters behave as documented in L<nbd_flush(3)>." +^ strict_call_description; see_also = [SectionLink "Issuing asynchronous commands"; - Link "can_flush"; Link "flush"]; + Link "can_flush"; Link "flush"; Link "set_strict_mode"]; }; "aio_trim", { @@ -2152,9 +2232,10 @@ To check if the command completed, call L<nbd_aio_command_completed(3)>. Or supply the optional C<completion_callback> which will be invoked as described in L<libnbd(3)/Completion callbacks>. -Other parameters behave as documented in L<nbd_trim(3)>."; +Other parameters behave as documented in L<nbd_trim(3)>." +^ strict_call_description; see_also = [SectionLink "Issuing asynchronous commands"; - Link "can_trim"; Link "trim"]; + Link "can_trim"; Link "trim"; Link "set_strict_mode"]; }; "aio_cache", { @@ -2172,9 +2253,10 @@ To check if the command completed, call L<nbd_aio_command_completed(3)>. Or supply the optional C<completion_callback> which will be invoked as described in L<libnbd(3)/Completion callbacks>. -Other parameters behave as documented in L<nbd_cache(3)>."; +Other parameters behave as documented in L<nbd_cache(3)>." +^ strict_call_description; see_also = [SectionLink "Issuing asynchronous commands"; - Link "can_cache"; Link "cache"]; + Link "can_cache"; Link "cache"; Link "set_strict_mode"]; }; "aio_zero", { @@ -2193,10 +2275,11 @@ To check if the command completed, call L<nbd_aio_command_completed(3)>. Or supply the optional C<completion_callback> which will be invoked as described in L<libnbd(3)/Completion callbacks>. -Other parameters behave as documented in L<nbd_zero(3)>."; +Other parameters behave as documented in L<nbd_zero(3)>." +^ strict_call_description; see_also = [SectionLink "Issuing asynchronous commands"; Link "can_zero"; Link "can_fast_zero"; - Link "zero"]; + Link "zero"; Link "set_strict_mode"]; }; "aio_block_status", { @@ -2214,9 +2297,11 @@ To check if the command completed, call L<nbd_aio_command_completed(3)>. Or supply the optional C<completion_callback> which will be invoked as described in L<libnbd(3)/Completion callbacks>. -Other parameters behave as documented in L<nbd_block_status(3)>."; +Other parameters behave as documented in L<nbd_block_status(3)>." +^ strict_call_description; see_also = [SectionLink "Issuing asynchronous commands"; - Link "can_meta_context"; Link "block_status"]; + Link "can_meta_context"; Link "block_status"; + Link "set_strict_mode"]; }; "aio_get_fd", { @@ -2663,6 +2748,10 @@ let first_version = [ "aio_opt_list", (1, 4); "aio_opt_info", (1, 4); + (* Added in 1.5.x development cycle, will be stable and supported in 1.6. *) + "set_strict_mode", (1, 6); + "get_strict_mode", (1, 6); + (* These calls are proposed for a future version of libnbd, but * have not been added to any released version so far. "get_tls_certificates", (1, ??); diff --git a/lib/handle.c b/lib/handle.c index 4d26842..2dc1fb5 100644 --- a/lib/handle.c +++ b/lib/handle.c @@ -74,6 +74,8 @@ nbd_create (void) s = getenv ("LIBNBD_DEBUG"); h->debug = s && strcmp (s, "1") == 0; + h->strict = LIBNBD_STRICT_MASK; + h->public_state = STATE_START; h->state = STATE_START; h->pid = -1; @@ -349,6 +351,20 @@ nbd_unlocked_get_handshake_flags (struct nbd_handle *h) return h->gflags; } +int +nbd_unlocked_set_strict_mode (struct nbd_handle *h, uint32_t flags) +{ + h->strict = flags; + return 0; +} + +/* NB: may_set_error = false. */ +uint32_t +nbd_unlocked_get_strict_mode (struct nbd_handle *h) +{ + return h->strict; +} + const char * nbd_unlocked_get_package_name (struct nbd_handle *h) { diff --git a/lib/rw.c b/lib/rw.c index 95c002c..f49fe25 100644 --- a/lib/rw.c +++ b/lib/rw.c @@ -295,10 +295,12 @@ nbd_unlocked_aio_pread_structured (struct nbd_handle *h, void *buf, struct command_cb cb = { .fn.chunk = *chunk, .completion = *completion }; - if ((flags & LIBNBD_CMD_FLAG_DF) != 0 && - nbd_unlocked_can_df (h) != 1) { - set_error (EINVAL, "server does not support the DF flag"); - return -1; + if (h->strict & LIBNBD_STRICT_COMMANDS) { + if ((flags & LIBNBD_CMD_FLAG_DF) != 0 && + nbd_unlocked_can_df (h) != 1) { + set_error (EINVAL, "server does not support the DF flag"); + return -1; + } } SET_CALLBACK_TO_NULL (*chunk); @@ -315,15 +317,17 @@ nbd_unlocked_aio_pwrite (struct nbd_handle *h, const void *buf, { struct command_cb cb = { .completion = *completion }; - if (nbd_unlocked_is_read_only (h) == 1) { - set_error (EPERM, "server does not support write operations"); - return -1; - } + if (h->strict & LIBNBD_STRICT_COMMANDS) { + if (nbd_unlocked_is_read_only (h) == 1) { + set_error (EPERM, "server does not support write operations"); + return -1; + } - if ((flags & LIBNBD_CMD_FLAG_FUA) != 0 && - nbd_unlocked_can_fua (h) != 1) { - set_error (EINVAL, "server does not support the FUA flag"); - return -1; + if ((flags & LIBNBD_CMD_FLAG_FUA) != 0 && + nbd_unlocked_can_fua (h) != 1) { + set_error (EINVAL, "server does not support the FUA flag"); + return -1; + } } SET_CALLBACK_TO_NULL (*completion); @@ -338,9 +342,11 @@ nbd_unlocked_aio_flush (struct nbd_handle *h, { struct command_cb cb = { .completion = *completion }; - if (nbd_unlocked_can_flush (h) != 1) { - set_error (EINVAL, "server does not support flush operations"); - return -1; + if (h->strict & LIBNBD_STRICT_COMMANDS) { + if (nbd_unlocked_can_flush (h) != 1) { + set_error (EINVAL, "server does not support flush operations"); + return -1; + } } SET_CALLBACK_TO_NULL (*completion); @@ -356,19 +362,21 @@ nbd_unlocked_aio_trim (struct nbd_handle *h, { struct command_cb cb = { .completion = *completion }; - if (nbd_unlocked_can_trim (h) != 1) { - set_error (EINVAL, "server does not support trim operations"); - return -1; - } - if (nbd_unlocked_is_read_only (h) == 1) { - set_error (EPERM, "server does not support write operations"); - return -1; - } + if (h->strict & LIBNBD_STRICT_COMMANDS) { + if (nbd_unlocked_can_trim (h) != 1) { + set_error (EINVAL, "server does not support trim operations"); + return -1; + } + if (nbd_unlocked_is_read_only (h) == 1) { + set_error (EPERM, "server does not support write operations"); + return -1; + } - if ((flags & LIBNBD_CMD_FLAG_FUA) != 0 && - nbd_unlocked_can_fua (h) != 1) { - set_error (EINVAL, "server does not support the FUA flag"); - return -1; + if ((flags & LIBNBD_CMD_FLAG_FUA) != 0 && + nbd_unlocked_can_fua (h) != 1) { + set_error (EINVAL, "server does not support the FUA flag"); + return -1; + } } if (count == 0) { /* NBD protocol forbids this. */ @@ -389,13 +397,15 @@ nbd_unlocked_aio_cache (struct nbd_handle *h, { struct command_cb cb = { .completion = *completion }; - /* Actually according to the NBD protocol document, servers do exist - * that support NBD_CMD_CACHE but don't advertise the - * NBD_FLAG_SEND_CACHE bit, but we ignore those. - */ - if (nbd_unlocked_can_cache (h) != 1) { - set_error (EINVAL, "server does not support cache operations"); - return -1; + if (h->strict & LIBNBD_STRICT_COMMANDS) { + /* Actually according to the NBD protocol document, servers do exist + * that support NBD_CMD_CACHE but don't advertise the + * NBD_FLAG_SEND_CACHE bit, but we ignore those. + */ + if (nbd_unlocked_can_cache (h) != 1) { + set_error (EINVAL, "server does not support cache operations"); + return -1; + } } SET_CALLBACK_TO_NULL (*completion); @@ -411,25 +421,27 @@ nbd_unlocked_aio_zero (struct nbd_handle *h, { struct command_cb cb = { .completion = *completion }; - if (nbd_unlocked_can_zero (h) != 1) { - set_error (EINVAL, "server does not support zero operations"); - return -1; - } - if (nbd_unlocked_is_read_only (h) == 1) { - set_error (EPERM, "server does not support write operations"); - return -1; - } + if (h->strict & LIBNBD_STRICT_COMMANDS) { + if (nbd_unlocked_can_zero (h) != 1) { + set_error (EINVAL, "server does not support zero operations"); + return -1; + } + if (nbd_unlocked_is_read_only (h) == 1) { + set_error (EPERM, "server does not support write operations"); + return -1; + } - if ((flags & LIBNBD_CMD_FLAG_FUA) != 0 && - nbd_unlocked_can_fua (h) != 1) { - set_error (EINVAL, "server does not support the FUA flag"); - return -1; - } + if ((flags & LIBNBD_CMD_FLAG_FUA) != 0 && + nbd_unlocked_can_fua (h) != 1) { + set_error (EINVAL, "server does not support the FUA flag"); + return -1; + } - if ((flags & LIBNBD_CMD_FLAG_FAST_ZERO) != 0 && - nbd_unlocked_can_fast_zero (h) != 1) { - set_error (EINVAL, "server does not support the fast zero flag"); - return -1; + if ((flags & LIBNBD_CMD_FLAG_FAST_ZERO) != 0 && + nbd_unlocked_can_fast_zero (h) != 1) { + set_error (EINVAL, "server does not support the fast zero flag"); + return -1; + } } if (count == 0) { /* NBD protocol forbids this. */ @@ -452,16 +464,18 @@ nbd_unlocked_aio_block_status (struct nbd_handle *h, struct command_cb cb = { .fn.extent = *extent, .completion = *completion }; - if (!h->structured_replies) { - set_error (ENOTSUP, "server does not support structured replies"); - return -1; - } + if (h->strict & LIBNBD_STRICT_COMMANDS) { + if (!h->structured_replies) { + set_error (ENOTSUP, "server does not support structured replies"); + return -1; + } - if (h->meta_contexts == NULL) { - set_error (ENOTSUP, "did not negotiate any metadata contexts, " - "either you did not call nbd_add_meta_context before " - "connecting or the server does not support it"); - return -1; + if (h->meta_contexts == NULL) { + set_error (ENOTSUP, "did not negotiate any metadata contexts, " + "either you did not call nbd_add_meta_context before " + "connecting or the server does not support it"); + return -1; + } } if (count == 0) { /* NBD protocol forbids this. */ diff --git a/tests/errors.c b/tests/errors.c index 8166429..0c4151a 100644 --- a/tests/errors.c +++ b/tests/errors.c @@ -86,6 +86,7 @@ main (int argc, char *argv[]) */ const char *cmd[] = { "nbdkit", "-s", "--exit-with-parent", "sh", script, NULL }; + uint32_t strict; progname = argv[0]; @@ -269,7 +270,25 @@ main (int argc, char *argv[]) } check (ERANGE, "nbd_aio_pwrite: "); - /* Use unadvertised command */ + /* Use unadvertised command, client-side */ + strict = nbd_get_strict_mode (nbd) | LIBNBD_STRICT_COMMANDS; + if (nbd_set_strict_mode (nbd, strict) == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + if (nbd_trim (nbd, 512, 0, 0) != -1) { + fprintf (stderr, "%s: test failed: " + "unpermitted nbd_trim did not fail\n", + argv[0]); + exit (EXIT_FAILURE); + } + check (EINVAL, "nbd_trim: "); + /* Use unadvertised command, server-side */ + strict &= ~LIBNBD_STRICT_COMMANDS; + if (nbd_set_strict_mode (nbd, strict) == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } if (nbd_trim (nbd, 512, 0, 0) != -1) { fprintf (stderr, "%s: test failed: " "unpermitted nbd_trim did not fail\n", -- 2.28.0