Add APIs to make it possible to request and display NBD_INFO_NAME and NBD_INFO_DESCRIPTION, which will enable fixing one of the todo items in nbdinfo when not used in --list mode. Note that at least qemu-nbd does not advertise name and description during NBD_OPT_GO unless the client requests it first; at the same time, always requesting it only slows down handshaking, so exposing a knob seems better than always doing this check (contrast that with NBD_INFO_BLOCK_SIZE, which affects correctness of data access). --- lib/internal.h | 13 ++++- lib/nbd-protocol.h | 10 +++- generator/API.ml | 81 +++++++++++++++++++++++++++++- generator/states-newstyle-opt-go.c | 49 +++++++++++++++--- lib/handle.c | 61 ++++++++++++++++++++++ 5 files changed, 204 insertions(+), 10 deletions(-) diff --git a/lib/internal.h b/lib/internal.h index 6106af3..aa7e9a2 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -104,6 +104,9 @@ struct nbd_handle { size_t nr_exports; struct export *exports; + /* Full info mode. */ + bool full_info; + /* Global flags from the server. */ uint16_t gflags; @@ -120,6 +123,10 @@ struct nbd_handle { uint32_t block_preferred; uint32_t block_maximum; + /* Server canonical name and description, or NULL if not advertised. */ + char *canonical_name; + char *description; + /* Flags set by the state machine to tell what protocol and whether * TLS was negotiated. */ @@ -180,6 +187,10 @@ struct nbd_handle { } __attribute__((packed)) server; struct nbd_fixed_new_option_reply_info_export export; struct nbd_fixed_new_option_reply_info_block_size block_size; + struct { + struct nbd_fixed_new_option_reply_info_name_or_desc info; + char str[NBD_MAX_STRING]; + } __attribute__((packed)) name_desc; struct { struct nbd_fixed_new_option_reply_meta_context context; char str[NBD_MAX_STRING]; @@ -205,7 +216,7 @@ struct nbd_handle { uint32_t cflags; uint32_t len; uint16_t nrinfos; - uint16_t info; + uint16_t info[3]; uint32_t nrqueries; } sbuf; diff --git a/lib/nbd-protocol.h b/lib/nbd-protocol.h index 627bc6e..e5d6404 100644 --- a/lib/nbd-protocol.h +++ b/lib/nbd-protocol.h @@ -154,6 +154,14 @@ struct nbd_fixed_new_option_reply_info_export { uint16_t eflags; /* per-export flags */ } NBD_ATTRIBUTE_PACKED; +/* NBD_INFO_NAME or NBD_INFO_DESCRIPTION reply (follows + * fixed_new_option_reply). + */ +struct nbd_fixed_new_option_reply_info_name_or_desc { + uint16_t info; /* NBD_INFO_NAME, NBD_INFO_DESCRIPTION */ + /* followed by a string name or description */ +} NBD_ATTRIBUTE_PACKED; + /* NBD_INFO_BLOCK_SIZE reply (follows fixed_new_option_reply). */ struct nbd_fixed_new_option_reply_info_block_size { uint16_t info; /* NBD_INFO_BLOCK_SIZE */ @@ -165,7 +173,7 @@ struct nbd_fixed_new_option_reply_info_block_size { /* NBD_REP_SERVER reply (follows fixed_new_option_reply). */ struct nbd_fixed_new_option_reply_server { uint32_t export_name_len; /* length of export name */ - /* followed by a string export name and description*/ + /* followed by a string export name and description */ } NBD_ATTRIBUTE_PACKED; /* NBD_REP_META_CONTEXT reply (follows fixed_new_option_reply). */ diff --git a/generator/API.ml b/generator/API.ml index 2f3b57d..eff0ac4 100644 --- a/generator/API.ml +++ b/generator/API.ml @@ -294,8 +294,81 @@ to a URI that includes an export name."; args = []; ret = RString; shortdesc = "get the export name"; longdesc = "\ -Get the export name associated with the handle."; - see_also = [Link "set_export_name"; Link "connect_uri"]; +Get the export name associated with the handle. This is the name +that libnbd requests; see L<nbd_get_canonical_export_name(3)> for +determining if the server has a different canonical name for the +given export (most common when requesting the default export name +of an empty string C<\"\">)"; + see_also = [Link "set_export_name"; Link "connect_uri"; + Link "get_canonical_export_name"]; + }; + + "set_full_info", { + default_call with + args = [Bool "request"]; ret = RErr; + permitted_states = [ Created ]; + shortdesc = "control whether NBD_OPT_GO requests extra details"; + longdesc = "\ +By default, when connecting to an export, libnbd only requests the +details it needs to service data operations. The NBD server says +that a server can supply optional information, such as a canonical +name of the export (see L<nbd_get_canonical_export_name(3)>) or +a description of the export (see L<nbd_get_export_description(3)>), +but that a hint from the client makes it more likely for this +extra information to be provided. This function controls whether +libnbd will provide that hint. + +Note that even when full info is requested, the server is not +obligated to reply with all information that libnbd requested. +Similarly, libnbd will ignore any optional server information that +libnbd has not yet been taught to recognize."; + see_also = [Link "get_full_info"; Link "get_canonical_export_name"; + Link "get_export_description"]; + }; + + "get_full_info", { + default_call with + args = []; ret = RBool; + permitted_states = []; + shortdesc = "see if NBD_OPT_GO requests extra details"; + longdesc = "\ +Return the state of the full info request flag on this handle."; + see_also = [Link "set_full_info"]; + }; + + "get_canonical_export_name", { + default_call with + args = []; ret = RString; + permitted_states = [ Connected; Closed ]; + shortdesc = "return the canonical export name, if the server has one"; + longdesc = "\ +The NBD protocol permits a server to report an optional canonical +export name which may differ from the client's request (as set by +L<nbd_set_export_name(3)> or L<nbd_connect_uri(3)>. This function +accesses any name returned by the server; it may be the same as +the client request, but is more likely to differ when the client +requested a connection to the default export name (an empty string +C<\"\">). + +Some servers are unlikely to report a canonical name unless the +client specifically hinted about wanting it, via L<nbd_set_full_info(3)>."; + see_also = [Link "set_full_info"; Link "get_export_name"]; + }; + + "get_export_description", { + default_call with + args = []; ret = RString; + permitted_states = [ Connected; Closed ]; + shortdesc = "return the export description, if the server has one"; + longdesc = "\ +The NBD protocol permits a server to report an optional export +description. This function reports any description reported by +the server. + +Some servers are unlikely to report description unless the +client specifically hinted about wanting it, via L<nbd_set_full_info(3)>. +For L<qemu-nbd(8)>, a description is set with I<-D>."; + see_also = [Link "set_full_info"]; }; "set_tls", { @@ -2364,6 +2437,10 @@ let first_version = [ "get_list_export_name", (1, 4); "get_list_export_description", (1, 4); "get_block_size", (1, 4); + "set_full_info", (1, 4); + "get_full_info", (1, 4); + "get_canonical_export_name", (1, 4); + "get_export_description", (1, 4); (* These calls are proposed for a future version of libnbd, but * have not been added to any released version so far. diff --git a/generator/states-newstyle-opt-go.c b/generator/states-newstyle-opt-go.c index 99a1dcd..74f092e 100644 --- a/generator/states-newstyle-opt-go.c +++ b/generator/states-newstyle-opt-go.c @@ -20,11 +20,13 @@ STATE_MACHINE { NEWSTYLE.OPT_GO.START: + uint16_t nrinfos = h->full_info ? 3 : 1; + h->sbuf.option.version = htobe64 (NBD_NEW_VERSION); h->sbuf.option.option = htobe32 (NBD_OPT_GO); h->sbuf.option.optlen = htobe32 (/* exportnamelen */ 4 + strlen (h->export_name) - + /* nrinfos */ 2 + /* INFO_BLOCK_SIZE */ 2); + + sizeof nrinfos + 2 * nrinfos); h->wbuf = &h->sbuf; h->wlen = sizeof h->sbuf.option; h->wflags = MSG_MORE; @@ -57,23 +59,29 @@ STATE_MACHINE { return 0; NEWSTYLE.OPT_GO.SEND_EXPORT: + uint16_t nrinfos = h->full_info ? 3 : 1; + switch (send_from_wbuf (h)) { case -1: SET_NEXT_STATE (%.DEAD); return 0; case 0: - h->sbuf.nrinfos = htobe16 (1); + h->sbuf.nrinfos = htobe16 (nrinfos); h->wbuf = &h->sbuf; - h->wlen = 2; + h->wlen = sizeof h->sbuf.nrinfos; SET_NEXT_STATE (%SEND_NRINFOS); } return 0; NEWSTYLE.OPT_GO.SEND_NRINFOS: + uint16_t nrinfos = h->full_info ? 3 : 1; + switch (send_from_wbuf (h)) { case -1: SET_NEXT_STATE (%.DEAD); return 0; case 0: - h->sbuf.info = htobe16 (NBD_INFO_BLOCK_SIZE); + h->sbuf.info[0] = htobe16 (NBD_INFO_BLOCK_SIZE); + h->sbuf.info[1] = htobe16 (NBD_INFO_NAME); + h->sbuf.info[2] = htobe16 (NBD_INFO_DESCRIPTION); h->wbuf = &h->sbuf; - h->wlen = 2; + h->wlen = sizeof h->sbuf.info[0] * nrinfos; SET_NEXT_STATE (%SEND_INFO); } return 0; @@ -154,8 +162,37 @@ STATE_MACHINE { return 0; } break; + case NBD_INFO_NAME: + if (len > sizeof h->sbuf.or.payload.name_desc.info + NBD_MAX_STRING || + len < sizeof h->sbuf.or.payload.name_desc.info) { + SET_NEXT_STATE (%.DEAD); + set_error (0, "handshake: incorrect NBD_INFO_NAME option reply length"); + return 0; + } + free (h->canonical_name); + h->canonical_name = strndup (h->sbuf.or.payload.name_desc.str, len - 2); + if (h->canonical_name == NULL) { + SET_NEXT_STATE (%.DEAD); + set_error (errno, "strndup"); + return 0; + } + break; + case NBD_INFO_DESCRIPTION: + if (len > sizeof h->sbuf.or.payload.name_desc.info + NBD_MAX_STRING || + len < sizeof h->sbuf.or.payload.name_desc.info) { + SET_NEXT_STATE (%.DEAD); + set_error (0, "handshake: incorrect NBD_INFO_DESCRIPTION option reply length"); + return 0; + } + free (h->description); + h->description = strndup (h->sbuf.or.payload.name_desc.str, len - 2); + if (h->description == NULL) { + SET_NEXT_STATE (%.DEAD); + set_error (errno, "strndup"); + return 0; + } + break; default: - /* XXX Handle other info types, like NBD_INFO_DESCRIPTION */ debug (h, "skipping unknown NBD_REP_INFO type %d", be16toh (h->sbuf.or.payload.export.info)); break; diff --git a/lib/handle.c b/lib/handle.c index 210ac7d..28c30dd 100644 --- a/lib/handle.c +++ b/lib/handle.c @@ -136,6 +136,8 @@ nbd_close (struct nbd_handle *h) free (h->exports[i].description); } free (h->exports); + free (h->canonical_name); + free (h->description); free_cmd_list (h->cmds_to_issue); free_cmd_list (h->cmds_in_flight); free_cmd_list (h->cmds_done); @@ -232,6 +234,65 @@ nbd_unlocked_get_export_name (struct nbd_handle *h) return copy; } +int +nbd_unlocked_set_full_info (struct nbd_handle *h, bool request) +{ + h->full_info = request; + return 0; +} + +int +nbd_unlocked_get_full_info (struct nbd_handle *h) +{ + return h->full_info; +} + +char * +nbd_unlocked_get_canonical_export_name (struct nbd_handle *h) +{ + char *r; + + if (h->eflags == 0) { + set_error (EINVAL, "server has not returned export flags, " + "you need to connect to the server first"); + return NULL; + } + if (h->canonical_name == NULL) { + set_error (ENOTSUP, "server did not advertise a canonical name"); + return NULL; + } + + r = strdup (h->canonical_name); + if (r == NULL) { + set_error (errno, "strdup"); + return NULL; + } + return r; +} + +char * +nbd_unlocked_get_export_description (struct nbd_handle *h) +{ + char *r; + + if (h->eflags == 0) { + set_error (EINVAL, "server has not returned export flags, " + "you need to connect to the server first"); + return NULL; + } + if (h->description == NULL) { + set_error (ENOTSUP, "server did not advertise a description"); + return NULL; + } + + r = strdup (h->description); + if (r == NULL) { + set_error (errno, "strdup"); + return NULL; + } + return r; +} + int nbd_unlocked_set_list_exports (struct nbd_handle *h, bool list) { -- 2.28.0

This patch prepares for a change to export list extraction by adding a new state in the state machine, NEGOTIATING. At present, it is not actually possible to enter this state. But upcoming patches will allow the user to opt-in to fine-grained control over negotiation during the connecting phase. Since the handshake phase is synchronous (the client sends a full command, then waits for the full server response, with no interleaving or out-of-order traffic), the idea is that we can drop out of connecting over to Negotiating any time we are waiting for the user to determine the next client command to send, and then back to connecting until that option is handled; where unlike the data phase, we won't need a cookie. Other upcoming patches will then adjust existing API that make sense to call while in this state, as well as adding new API to give the user control over when to actually use this state. --- lib/internal.h | 1 + generator/API.ml | 17 +++++++++++++++++ generator/API.mli | 1 + generator/C.ml | 4 +++- generator/state_machine.ml | 7 +++++++ lib/connect.c | 5 +++-- lib/is-state.c | 14 +++++++++++++- 7 files changed, 45 insertions(+), 4 deletions(-) diff --git a/lib/internal.h b/lib/internal.h index aa7e9a2..186d677 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -410,6 +410,7 @@ extern int nbd_internal_set_block_size (struct nbd_handle *h, uint32_t min, /* is-state.c */ extern bool nbd_internal_is_state_created (enum state state); extern bool nbd_internal_is_state_connecting (enum state state); +extern bool nbd_internal_is_state_negotiating (enum state state); extern bool nbd_internal_is_state_ready (enum state state); extern bool nbd_internal_is_state_processing (enum state state); extern bool nbd_internal_is_state_dead (enum state state); diff --git a/generator/API.ml b/generator/API.ml index 0c8ef30..0990d48 100644 --- a/generator/API.ml +++ b/generator/API.ml @@ -90,6 +90,7 @@ and flags = { and permitted_state = | Created | Connecting +| Negotiating | Connected | Closed | Dead and link = @@ -2136,6 +2137,21 @@ issue commands (see L<nbd_aio_is_ready(3)>)."; see_also = [Link "aio_is_ready"]; }; + "aio_is_negotiating", { + default_call with + args = []; ret = RBool; is_locked = false; may_set_error = false; + shortdesc = "check if connection is ready to send handshake option"; + longdesc = "\ +Return true if this connection is ready to start another option +negotiation command while handshaking with the server. An option +command will move back to the connecting state (see +L<nbd_aio_is_connecting(3)>). Note that this state cannot be +reached unless requested by nbd_set_opt_mode, and even then +it only works with newstyle servers; an oldstyle server will skip +straight to L<nbd_aio_is_ready(3)>."; + see_also = [Link "aio_is_connecting"; Link "aio_is_ready"]; + }; + "aio_is_ready", { default_call with args = []; ret = RBool; is_locked = false; may_set_error = false; @@ -2444,6 +2460,7 @@ let first_version = [ "get_full_info", (1, 4); "get_canonical_export_name", (1, 4); "get_export_description", (1, 4); + "aio_is_negotiating", (1, 4); (* These calls are proposed for a future version of libnbd, but * have not been added to any released version so far. diff --git a/generator/API.mli b/generator/API.mli index 3dbd1ff..712e837 100644 --- a/generator/API.mli +++ b/generator/API.mli @@ -102,6 +102,7 @@ and flags = { and permitted_state = | Created (** can be called in the START state *) | Connecting (** can be called when connecting/handshaking *) +| Negotiating (** can be called when handshaking in opt_mode *) | Connected (** when connected and READY or processing, but not including CLOSED or DEAD *) | Closed | Dead (** can be called when the handle is diff --git a/generator/C.ml b/generator/C.ml index afb8142..6b65f6e 100644 --- a/generator/C.ml +++ b/generator/C.ml @@ -397,7 +397,8 @@ let permitted_state_text permitted_states = function | Created -> "newly created" | Connecting -> "connecting" - | Connected -> "connected and finished handshaking with the server" + | Negotiating -> "negotiating" + | Connected -> "connected with the server" | Closed -> "shut down" | Dead -> "dead" ) permitted_states @@ -421,6 +422,7 @@ let generate_lib_api_c () = function | Created -> "nbd_internal_is_state_created (state)" | Connecting -> "nbd_internal_is_state_connecting (state)" + | Negotiating -> "nbd_internal_is_state_negotiating (state)" | Connected -> "nbd_internal_is_state_ready (state) || nbd_internal_is_state_processing (state)" | Closed -> "nbd_internal_is_state_closed (state)" | Dead -> "nbd_internal_is_state_dead (state)" diff --git a/generator/state_machine.ml b/generator/state_machine.ml index 144a5f8..8525520 100644 --- a/generator/state_machine.ml +++ b/generator/state_machine.ml @@ -100,6 +100,13 @@ let rec state_machine = [ Group ("OLDSTYLE", oldstyle_state_machine); Group ("NEWSTYLE", newstyle_state_machine); + State { + default_state with + name = "NEGOTIATING"; + comment = "Connection is ready to negotiate an NBD option"; + external_events = [ CmdIssue, "NEWSTYLE.START" ]; + }; + State { default_state with name = "READY"; diff --git a/lib/connect.c b/lib/connect.c index 92ed6b1..7e42b14 100644 --- a/lib/connect.c +++ b/lib/connect.c @@ -1,5 +1,5 @@ /* NBD client library in userspace - * Copyright (C) 2013-2019 Red Hat Inc. + * Copyright (C) 2013-2020 Red Hat Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -40,7 +40,8 @@ static int error_unless_ready (struct nbd_handle *h) { - if (nbd_internal_is_state_ready (get_next_state (h))) + if (nbd_internal_is_state_ready (get_next_state (h)) || + nbd_internal_is_state_negotiating (get_next_state (h))) return 0; /* Why did it fail? */ diff --git a/lib/is-state.c b/lib/is-state.c index 1a85c7a..e019e53 100644 --- a/lib/is-state.c +++ b/lib/is-state.c @@ -1,5 +1,5 @@ /* NBD client library in userspace - * Copyright (C) 2013-2019 Red Hat Inc. + * Copyright (C) 2013-2020 Red Hat Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -58,6 +58,12 @@ nbd_internal_is_state_connecting (enum state state) return is_connecting_group (group); } +bool +nbd_internal_is_state_negotiating (enum state state) +{ + return state == STATE_NEGOTIATING; +} + bool nbd_internal_is_state_ready (enum state state) { @@ -120,6 +126,12 @@ nbd_unlocked_aio_is_connecting (struct nbd_handle *h) return nbd_internal_is_state_connecting (get_public_state (h)); } +int +nbd_unlocked_aio_is_negotiating (struct nbd_handle *h) +{ + return nbd_internal_is_state_negotiating (get_public_state (h)); +} + int nbd_unlocked_aio_is_ready (struct nbd_handle *h) { -- 2.28.0

Now that we have a new NEGOTIATING state, we need a knob for the user to request moving into that state. This patch does not actually alter the state machine, but one thing at a time. However, the libnbd docs now mention several commands that will be added in future commits. (Hmm - our generator refuses to compile in-tree links in API.ml that don't resolve, but not links in libnbd.pod.) One thing we can test at this point: an oldstyle server will never get into opt mode; that will only be possible for a newstyle server. --- docs/libnbd.pod | 38 ++++++++++++++- lib/internal.h | 3 ++ generator/API.ml | 46 +++++++++++++++++-- generator/states-newstyle-opt-go.c | 1 + generator/states-newstyle-opt-list.c | 1 + .../states-newstyle-opt-set-meta-context.c | 3 +- generator/states-newstyle-opt-starttls.c | 1 + .../states-newstyle-opt-structured-reply.c | 1 + lib/opt.c | 41 +++++++++++++++++ lib/Makefile.am | 3 +- tests/oldstyle.c | 29 ++++++++---- 11 files changed, 151 insertions(+), 16 deletions(-) create mode 100644 lib/opt.c diff --git a/docs/libnbd.pod b/docs/libnbd.pod index fc66888..4ee0815 100644 --- a/docs/libnbd.pod +++ b/docs/libnbd.pod @@ -447,6 +447,40 @@ subprocess. This works very similarly to L<nbd_connect_command(3)> described above, but you must use L<nbd_connect_systemd_socket_activation(3)> instead. +=head1 CONTROLLING NEGOTIATION + +By default, when beginning a connection, libnbd will handle all +negotiation with the server, using only the configuration +(eg. L<nbd_set_export_name(3)> or L<nbd_add_meta_context(3)>) that was +requested before the connection attempt; this phase continues until +L<nbd_aio_is_connecting(3)> no longer returns true, at which point, +either data commands are ready to use or else the connection has +failed with an error. + +But there are scenarios in which it is useful to also control the +handshaking commands sent during negotiation, such as asking the +server for a list of available exports prior to selecting which one to +use. This is done by calling L<nbd_set_opt_mode(3)> before +connecting; then after requesting a connection, the state machine will +pause at L<nbd_aio_is_negotiating(3)> at any point that the user can +decide which handshake command to send next. Note that the +negotiation state is only reachable from newstyle servers; older +servers cannot negotiate and will progress all the way to the ready +state. + +When the negotiating state is reached, you can initiate option +commands such as L<nbd_opt_list(3)> or their asynchronous equivalents, +as well as alter configuration such as export name that previously had +to be set before connection. Since the NBD protocol does not allow +parallel negotiating commands, no cookie is involved, and you can +track completion of each command when the state is no longer +L<nbd_aio_is_connecting(3)>. If L<nbd_opt_go(3)> fails but the +connection is still live, you will be back in negotiation state, where +you can request a different export name and try again. Exiting the +negotiation state is only possible with a successful L<nbd_opt_go(3)> +which moves to the data phase, or L<nbd_opt_abort(3)> which performs a +clean shutdown of the connection by skipping the data phase. + =head1 EXPORTS AND FLAGS It is possible for NBD servers to serve different content on different @@ -562,8 +596,8 @@ L<https://github.com/libguestfs/libnbd/blob/master/interop/dirty-bitmap.c> =head2 Issuing multiple in-flight requests NBD servers which properly implement the specification can handle -multiple requests in flight over the same connection at the same time. -Libnbd supports this when using the low level API. +multiple data requests in flight over the same connection at the same +time. Libnbd supports this when using the low level API. To use it you simply issue more requests as needed (eg. using calls like L<nbd_aio_pread(3)>, L<nbd_aio_pwrite(3)>) without waiting for previous diff --git a/lib/internal.h b/lib/internal.h index 186d677..5f495fb 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -99,6 +99,9 @@ struct nbd_handle { int uri_allow_tls; bool uri_allow_local_file; + /* Option negotiation mode. */ + bool opt_mode; + /* List exports mode. */ bool list_exports; size_t nr_exports; diff --git a/generator/API.ml b/generator/API.ml index a363117..3dd94f6 100644 --- a/generator/API.ml +++ b/generator/API.ml @@ -681,6 +681,40 @@ the time of compilation."; Link "aio_is_created"; Link "aio_is_ready"]; }; + "set_opt_mode", { + default_call with + args = [Bool "enable"]; ret = RErr; + permitted_states = [ Created ]; + shortdesc = "control option mode, for pausing during option negotiation"; + longdesc = "\ +Set this flag to true in order to request that a connection command +C<nbd_connect_*> will pause for negotiation options rather than +proceeding all the way to the ready state, when communicating with a +newstyle server. This setting has no effect when connecting to an +oldstyle server. + +When option mode is enabled, you have fine-grained control over which +options are negotiated, compared to the default of the server +negotiating everything on your behalf using settings made before +starting the connection. To leave the mode and proceed on to the +ready state, you must use nbd_opt_go successfully; a failed +C<nbd_opt_go> returns to the negotiating state to allow a change of +export name before trying again. You may also use nbd_opt_abort +to end the connection without finishing negotiation."; + example = Some "examples/list-exports.c"; + see_also = [Link "get_opt_mode"; Link "aio_is_negotiating"]; + }; + + "get_opt_mode", { + default_call with + args = []; ret = RBool; + may_set_error = false; + shortdesc = "return whether option mode was enabled"; + longdesc = "\ +Return true if option negotiation mode was enabled on this handle."; + see_also = [Link "set_opt_mode"]; + }; + "set_list_exports", { default_call with args = [Bool "list"]; ret = RErr; @@ -889,7 +923,7 @@ the NBD URI. This call parses the URI and calls L<nbd_set_export_name(3)> and L<nbd_set_tls(3)> and other calls as needed, followed by L<nbd_connect_tcp(3)> or L<nbd_connect_unix(3)>. However, it is possible to override the -export name portion of a URI by using C<nbd_set_opt_mode> to +export name portion of a URI by using L<nbd_set_opt_mode(3)> to enable option mode, then using L<nbd_set_export_name(3)> as part of later negotiation. @@ -1032,7 +1066,8 @@ Support for URIs that require TLS will fail if libnbd was not compiled with gnutls; you can test whether this is the case with L<nbd_supports_tls(3)>."; see_also = [URLLink "https://github.com/NetworkBlockDevice/nbd/blob/master/doc/uri.md"; - Link "set_export_name"; Link "set_tls"]; + Link "set_export_name"; Link "set_tls"; + Link "set_opt_mode"]; }; "connect_unix", { @@ -2151,10 +2186,11 @@ Return true if this connection is ready to start another option negotiation command while handshaking with the server. An option command will move back to the connecting state (see L<nbd_aio_is_connecting(3)>). Note that this state cannot be -reached unless requested by nbd_set_opt_mode, and even then +reached unless requested by L<nbd_set_opt_mode(3)>, and even then it only works with newstyle servers; an oldstyle server will skip straight to L<nbd_aio_is_ready(3)>."; - see_also = [Link "aio_is_connecting"; Link "aio_is_ready"]; + see_also = [Link "aio_is_connecting"; Link "aio_is_ready"; + Link "set_opt_mode"]; }; "aio_is_ready", { @@ -2465,6 +2501,8 @@ let first_version = [ "get_full_info", (1, 4); "get_canonical_export_name", (1, 4); "get_export_description", (1, 4); + "set_opt_mode", (1, 4); + "get_opt_mode", (1, 4); "aio_is_negotiating", (1, 4); (* These calls are proposed for a future version of libnbd, but diff --git a/generator/states-newstyle-opt-go.c b/generator/states-newstyle-opt-go.c index 74f092e..d696cae 100644 --- a/generator/states-newstyle-opt-go.c +++ b/generator/states-newstyle-opt-go.c @@ -22,6 +22,7 @@ STATE_MACHINE { NEWSTYLE.OPT_GO.START: uint16_t nrinfos = h->full_info ? 3 : 1; + assert (h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE); h->sbuf.option.version = htobe64 (NBD_NEW_VERSION); h->sbuf.option.option = htobe32 (NBD_OPT_GO); h->sbuf.option.optlen = diff --git a/generator/states-newstyle-opt-list.c b/generator/states-newstyle-opt-list.c index 80fcb1b..f2846b6 100644 --- a/generator/states-newstyle-opt-list.c +++ b/generator/states-newstyle-opt-list.c @@ -23,6 +23,7 @@ STATE_MACHINE { NEWSTYLE.OPT_LIST.START: + assert (h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE); if (!h->list_exports) { SET_NEXT_STATE (%^OPT_STRUCTURED_REPLY.START); return 0; diff --git a/generator/states-newstyle-opt-set-meta-context.c b/generator/states-newstyle-opt-set-meta-context.c index 92fe26c..77fd022 100644 --- a/generator/states-newstyle-opt-set-meta-context.c +++ b/generator/states-newstyle-opt-set-meta-context.c @@ -1,5 +1,5 @@ /* nbd client library in userspace: state machine - * Copyright (C) 2013-2019 Red Hat Inc. + * Copyright (C) 2013-2020 Red Hat Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -27,6 +27,7 @@ STATE_MACHINE { * Also we skip the group if the client didn't request any metadata * contexts. */ + assert (h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE); if (!h->structured_replies || h->request_meta_contexts == NULL || nbd_internal_string_list_length (h->request_meta_contexts) == 0) { diff --git a/generator/states-newstyle-opt-starttls.c b/generator/states-newstyle-opt-starttls.c index 2d74e5f..7162c7a 100644 --- a/generator/states-newstyle-opt-starttls.c +++ b/generator/states-newstyle-opt-starttls.c @@ -20,6 +20,7 @@ STATE_MACHINE { NEWSTYLE.OPT_STARTTLS.START: + assert (h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE); /* If TLS was not requested we skip this option and go to the next one. */ if (h->tls == LIBNBD_TLS_DISABLE) { SET_NEXT_STATE (%^OPT_LIST.START); diff --git a/generator/states-newstyle-opt-structured-reply.c b/generator/states-newstyle-opt-structured-reply.c index cfe6e0d..58a76db 100644 --- a/generator/states-newstyle-opt-structured-reply.c +++ b/generator/states-newstyle-opt-structured-reply.c @@ -20,6 +20,7 @@ STATE_MACHINE { NEWSTYLE.OPT_STRUCTURED_REPLY.START: + assert (h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE); if (!h->request_sr) { SET_NEXT_STATE (%^OPT_SET_META_CONTEXT.START); return 0; diff --git a/lib/opt.c b/lib/opt.c new file mode 100644 index 0000000..306a2e9 --- /dev/null +++ b/lib/opt.c @@ -0,0 +1,41 @@ +/* NBD client library in userspace + * Copyright (C) 2020 Red Hat Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library 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 + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include <config.h> + +#include <stdlib.h> +#include <stdbool.h> +#include <errno.h> +#include <assert.h> +#include <limits.h> + +#include "internal.h" + +int +nbd_unlocked_set_opt_mode (struct nbd_handle *h, bool value) +{ + h->opt_mode = value; + return 0; +} + +/* NB: may_set_error = false. */ +int +nbd_unlocked_get_opt_mode (struct nbd_handle *h) +{ + return h->opt_mode; +} diff --git a/lib/Makefile.am b/lib/Makefile.am index 1c46c54..9fd6331 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -1,5 +1,5 @@ # nbd client library in userspace -# Copyright (C) 2013-2019 Red Hat Inc. +# Copyright (C) 2013-2020 Red Hat Inc. # # This library is free software; you can redistribute it and/or # modify it under the terms of the GNU Lesser General Public @@ -47,6 +47,7 @@ libnbd_la_SOURCES = \ internal.h \ is-state.c \ nbd-protocol.h \ + opt.c \ poll.c \ protocol.c \ rw.c \ diff --git a/tests/oldstyle.c b/tests/oldstyle.c index dc58d94..fd8bdc5 100644 --- a/tests/oldstyle.c +++ b/tests/oldstyle.c @@ -1,5 +1,5 @@ /* NBD client library in userspace - * Copyright (C) 2013-2019 Red Hat Inc. + * Copyright (C) 2013-2020 Red Hat Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -106,18 +106,26 @@ main (int argc, char *argv[]) } nbd_close (nbd); - /* Now for a working connection */ + /* Now for a working connection. Requesting an export name and opt_mode + * have no effects. + */ nbd = nbd_create (); - if (nbd == NULL) { - fprintf (stderr, "%s\n", nbd_get_error ()); - exit (EXIT_FAILURE); - } - if (nbd_connect_command (nbd, args) == -1) { + if (nbd == NULL || + nbd_set_opt_mode (nbd, true) == -1 || + nbd_add_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION) == -1 || + nbd_set_export_name (nbd, "ignored") == -1 || + nbd_connect_command (nbd, args) == -1) { fprintf (stderr, "%s\n", nbd_get_error ()); exit (EXIT_FAILURE); } - /* Protocol should be "oldstyle", with no structured replies. */ + /* Protocol should be "oldstyle", with no structured replies or meta + * contexts. + */ + if (nbd_aio_is_ready (nbd) != true) { + fprintf (stderr, "unexpected state after connection\n"); + exit (EXIT_FAILURE); + } s = nbd_get_protocol (nbd); if (strcmp (s, "oldstyle") != 0) { fprintf (stderr, @@ -129,6 +137,11 @@ main (int argc, char *argv[]) "incorrect structured replies %" PRId64 ", expected 0\n", r); exit (EXIT_FAILURE); } + if ((r = nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION)) != 0) { + fprintf (stderr, + "incorrect meta context %" PRId64 ", expected 0\n", r); + exit (EXIT_FAILURE); + } if ((r = nbd_get_size (nbd)) == -1) { fprintf (stderr, "%s\n", nbd_get_error ()); -- 2.28.0

Time to make option negotiation actually useful, by now allowing the user to request an export name before (re-)trying NBD_OPT_GO. This also means that we have to reset state before each attempt at NBD_OPT_GO (any meta contexts from the previous try must not be inherited into the next try). For now, I chose not to have a completion closure on nbd_aio_opt_go; you can detect success or failure based on the resulting state, and being able to set *err would be awkward (you can't magically get back to negotiation state if the server has moved on to data phase). But we have until 1.4 to decide if this decision needs to be revisited. --- lib/internal.h | 1 + generator/API.ml | 55 ++++++++++++++++--- generator/states-newstyle-opt-go.c | 6 +- .../states-newstyle-opt-set-meta-context.c | 1 + generator/states-newstyle.c | 6 ++ lib/flags.c | 28 +++++++++- lib/handle.c | 7 +-- lib/opt.c | 26 +++++++++ tests/newstyle-limited.c | 9 ++- 9 files changed, 122 insertions(+), 17 deletions(-) diff --git a/lib/internal.h b/lib/internal.h index 03baacd..3672538 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -405,6 +405,7 @@ extern void nbd_internal_set_last_error (int errnum, char *error); } while (0) /* flags.c */ +extern void nbd_internal_reset_size_and_flags (struct nbd_handle *h); extern int nbd_internal_set_size_and_flags (struct nbd_handle *h, uint64_t exportsize, uint16_t eflags); diff --git a/generator/API.ml b/generator/API.ml index cbc1c33..21485ea 100644 --- a/generator/API.ml +++ b/generator/API.ml @@ -285,9 +285,15 @@ export names. The NBD protocol limits export names to 4096 bytes, but servers may not support the full length. The encoding of export names is always UTF-8. +When option mode is not in use, the export name must be set +before beginning a connection. However, when L<nbd_set_opt_mode(3)> +has enabled option mode, it is possible to change the export +name prior to L<nbd_opt_go(3)>. + This call may be skipped if using L<nbd_connect_uri(3)> to connect to a URI that includes an export name."; - see_also = [Link "get_export_name"; Link "connect_uri"]; + see_also = [Link "get_export_name"; Link "connect_uri"; + Link "set_opt_mode"; Link "opt_go"]; }; "get_export_name", { @@ -697,13 +703,13 @@ When option mode is enabled, you have fine-grained control over which options are negotiated, compared to the default of the server negotiating everything on your behalf using settings made before starting the connection. To leave the mode and proceed on to the -ready state, you must use nbd_opt_go successfully; a failed -C<nbd_opt_go> returns to the negotiating state to allow a change of +ready state, you must use L<nbd_opt_go(3)> successfully; a failed +L<nbd_opt_go(3)> returns to the negotiating state to allow a change of export name before trying again. You may also use L<nbd_opt_abort(3)> to end the connection without finishing negotiation."; example = Some "examples/list-exports.c"; see_also = [Link "get_opt_mode"; Link "aio_is_negotiating"; - Link "opt_abort"]; + Link "opt_abort"; Link "opt_go"]; }; "get_opt_mode", { @@ -716,6 +722,24 @@ Return true if option negotiation mode was enabled on this handle."; see_also = [Link "set_opt_mode"]; }; + "opt_go", { + default_call with + args = []; ret = RErr; + permitted_states = [ Negotiating ]; + shortdesc = "end negotiation and move on to using an export"; + longdesc = "\ +Request that the server finish negotiation and move on to serving the +export previously specified by L<nbd_set_export_name(3)>. This can only +be used if L<nbd_set_opt_mode(3)> enabled option mode. + +If this fails, the server may still be in negotiation, where it is +possible to attempt another option such as a different export name; +although older servers will instead have killed the connection."; + example = Some "examples/list-exports.c"; + see_also = [Link "set_opt_mode"; Link "aio_opt_go"; Link "opt_abort"; + Link "set_export_name"]; + }; + "opt_abort", { default_call with args = []; ret = RErr; @@ -726,7 +750,7 @@ Request that the server finish negotiation, gracefully if possible, then close the connection. This can only be used if L<nbd_set_opt_mode(3)> enabled option mode."; example = Some "examples/list-exports.c"; - see_also = [Link "set_opt_mode"; Link "aio_opt_abort"]; + see_also = [Link "set_opt_mode"; Link "aio_opt_abort"; Link "opt_go"]; }; "set_list_exports", { @@ -938,8 +962,8 @@ L<nbd_set_export_name(3)> and L<nbd_set_tls(3)> and other calls as needed, followed by L<nbd_connect_tcp(3)> or L<nbd_connect_unix(3)>. However, it is possible to override the export name portion of a URI by using L<nbd_set_opt_mode(3)> to -enable option mode, then using L<nbd_set_export_name(3)> as part -of later negotiation. +enable option mode, then using L<nbd_set_export_name(3)> and +L<nbd_opt_go(3)> as part of subsequent negotiation. This call returns when the connection has been made. @@ -1897,6 +1921,21 @@ and completed the NBD handshake by calling L<nbd_aio_is_ready(3)>, on the connection."; }; + "aio_opt_go", { + default_call with + args = []; ret = RErr; + permitted_states = [ Negotiating ]; + shortdesc = "end negotiation and move on to using an export"; + longdesc = "\ +Request that the server finish negotiation and move on to serving the +export previously specified by L<nbd_set_export_name(3)>. This can only +be used if L<nbd_set_opt_mode(3)> enabled option mode. + +To determine when the request completes, wait for +L<nbd_aio_is_connecting(3)> to return false."; + see_also = [Link "set_opt_mode"; Link "opt_go"]; + }; + "aio_opt_abort", { default_call with args = []; ret = RErr; @@ -2533,7 +2572,9 @@ let first_version = [ "set_opt_mode", (1, 4); "get_opt_mode", (1, 4); "aio_is_negotiating", (1, 4); + "opt_go", (1, 4); "opt_abort", (1, 4); + "aio_opt_go", (1, 4); "aio_opt_abort", (1, 4); (* These calls are proposed for a future version of libnbd, but diff --git a/generator/states-newstyle-opt-go.c b/generator/states-newstyle-opt-go.c index 1a59ae7..0568695 100644 --- a/generator/states-newstyle-opt-go.c +++ b/generator/states-newstyle-opt-go.c @@ -240,7 +240,11 @@ STATE_MACHINE { set_error (0, "handshake: unknown reply from NBD_OPT_GO: 0x%" PRIx32, reply); } - SET_NEXT_STATE (%^PREPARE_OPT_ABORT); + nbd_internal_reset_size_and_flags (h); + if (h->opt_mode) + SET_NEXT_STATE (%.NEGOTIATING); + else + SET_NEXT_STATE (%^PREPARE_OPT_ABORT); break; case NBD_REP_ACK: SET_NEXT_STATE (%^FINISHED); diff --git a/generator/states-newstyle-opt-set-meta-context.c b/generator/states-newstyle-opt-set-meta-context.c index 77fd022..2dc8db8 100644 --- a/generator/states-newstyle-opt-set-meta-context.c +++ b/generator/states-newstyle-opt-set-meta-context.c @@ -28,6 +28,7 @@ STATE_MACHINE { * contexts. */ assert (h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE); + nbd_internal_reset_size_and_flags (h); if (!h->structured_replies || h->request_meta_contexts == NULL || nbd_internal_string_list_length (h->request_meta_contexts) == 0) { diff --git a/generator/states-newstyle.c b/generator/states-newstyle.c index 6613f9f..d771097 100644 --- a/generator/states-newstyle.c +++ b/generator/states-newstyle.c @@ -118,6 +118,12 @@ STATE_MACHINE { * state has informed us what we still need to do. */ switch (h->current_opt) { + case NBD_OPT_GO: + if ((h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE) == 0) + SET_NEXT_STATE (%OPT_EXPORT_NAME.START); + else + SET_NEXT_STATE (%OPT_SET_META_CONTEXT.START); + return 0; case NBD_OPT_ABORT: if ((h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE) == 0) { SET_NEXT_STATE (%.DEAD); diff --git a/lib/flags.c b/lib/flags.c index 223b77d..09eac72 100644 --- a/lib/flags.c +++ b/lib/flags.c @@ -27,6 +27,31 @@ #include "internal.h" +/* Reset connection data. Called after swapping export name, after + * failed OPT_GO/OPT_INFO, or when starting a fresh OPT_SET_META_CONTEXT. + */ +void +nbd_internal_reset_size_and_flags (struct nbd_handle *h) +{ + struct meta_context *m, *m_next; + + h->exportsize = 0; + h->eflags = 0; + for (m = h->meta_contexts; m != NULL; m = m_next) { + m_next = m->next; + free (m->name); + free (m); + } + h->meta_contexts = NULL; + h->block_minimum = 0; + h->block_preferred = 0; + h->block_maximum = 0; + free (h->canonical_name); + h->canonical_name = NULL; + free (h->description); + h->description = NULL; +} + /* Set the export size and eflags on the handle, validating them. * This is called from the state machine when either the newstyle or * oldstyle negotiation reaches the point that these are available. @@ -194,7 +219,8 @@ nbd_unlocked_get_size (struct nbd_handle *h) return h->exportsize; } -int64_t nbd_unlocked_get_block_size (struct nbd_handle *h, int type) +int64_t +nbd_unlocked_get_block_size (struct nbd_handle *h, int type) { switch (type) { case LIBNBD_SIZE_MINIMUM: diff --git a/lib/handle.c b/lib/handle.c index 28c30dd..a51b923 100644 --- a/lib/handle.c +++ b/lib/handle.c @@ -112,7 +112,6 @@ nbd_create (void) void nbd_close (struct nbd_handle *h) { - struct meta_context *m, *m_next; size_t i; nbd_internal_set_error_context ("nbd_close"); @@ -126,11 +125,7 @@ nbd_close (struct nbd_handle *h) nbd_unlocked_clear_debug_callback (h); free (h->bs_entries); - for (m = h->meta_contexts; m != NULL; m = m_next) { - m_next = m->next; - free (m->name); - free (m); - } + nbd_internal_reset_size_and_flags (h); for (i = 0; i < h->nr_exports; ++i) { free (h->exports[i].name); free (h->exports[i].description); diff --git a/lib/opt.c b/lib/opt.c index 6243553..6ad7f1a 100644 --- a/lib/opt.c +++ b/lib/opt.c @@ -51,6 +51,21 @@ wait_for_option (struct nbd_handle *h) return 0; } +/* Issue NBD_OPT_GO (or NBD_OPT_EXPORT_NAME) and wait for the reply. */ +int +nbd_unlocked_opt_go (struct nbd_handle *h) +{ + int r = nbd_unlocked_aio_opt_go (h); + + if (r == -1) + return r; + + r = wait_for_option (h); + if (r == 0 && nbd_internal_is_state_negotiating (get_next_state (h))) + return -1; /* NBD_OPT_GO failed, but can be attempted again */ + return r; +} + /* Issue NBD_OPT_ABORT and wait for the state change. */ int nbd_unlocked_opt_abort (struct nbd_handle *h) @@ -63,6 +78,17 @@ nbd_unlocked_opt_abort (struct nbd_handle *h) return wait_for_option (h); } +/* Issue NBD_OPT_GO (or NBD_OPT_EXPORT_NAME) without waiting. */ +int +nbd_unlocked_aio_opt_go (struct nbd_handle *h) +{ + h->current_opt = NBD_OPT_GO; + + if (nbd_internal_run (h, cmd_issue) == -1) + debug (h, "option queued, ignoring state machine failure"); + return 0; +} + /* Issue NBD_OPT_ABORT without waiting. */ int nbd_unlocked_aio_opt_abort (struct nbd_handle *h) diff --git a/tests/newstyle-limited.c b/tests/newstyle-limited.c index cdda474..aa6f87b 100644 --- a/tests/newstyle-limited.c +++ b/tests/newstyle-limited.c @@ -152,11 +152,16 @@ main (int argc, char *argv[]) */ nbd = nbd_create (); if (nbd == NULL || - nbd_set_export_name (nbd, "b") == -1 || - nbd_connect_command (nbd, args) == 1) { + nbd_set_opt_mode (nbd, true) == -1 || + nbd_connect_command (nbd, args) == -1 || + nbd_set_export_name (nbd, "b") == -1) { fprintf (stderr, "%s\n", nbd_get_error ()); exit (EXIT_FAILURE); } + if (nbd_opt_go (nbd) != -1) { + fprintf (stderr, "%s\n", "expected failure"); + exit (EXIT_FAILURE); + } if (!nbd_aio_is_dead (nbd)) { fprintf (stderr, "unexpected state after failed export\n"); exit (EXIT_FAILURE); -- 2.28.0

Instead of having to munge a URI to supply a different export name, we can exploit the fact that nbd_opt_mode lets us change the preferred export name after nbd_connect_uri has established its socket. In fact, by doing this, nbdinfo no longer needs to directly link against libxml2, so it can now be built regardless of whether the underlying libnbd.so supports URIs (although use of the program will fail if libnbd.so was not built against libxml2). --- info/Makefile.am | 6 ++--- info/nbdinfo.c | 63 +++++++++++++++++++----------------------------- 2 files changed, 27 insertions(+), 42 deletions(-) diff --git a/info/Makefile.am b/info/Makefile.am index 05b8137..d049d16 100644 --- a/info/Makefile.am +++ b/info/Makefile.am @@ -27,8 +27,6 @@ EXTRA_DIST = \ nbdinfo.pod \ $(NULL) -if HAVE_LIBXML2 - TESTS_ENVIRONMENT = LIBNBD_DEBUG=1 LOG_COMPILER = $(top_builddir)/run TESTS = @@ -39,11 +37,9 @@ nbdinfo_SOURCES = nbdinfo.c nbdinfo_CPPFLAGS = -I$(top_srcdir)/include nbdinfo_CFLAGS = \ $(WARNINGS_CFLAGS) \ - $(LIBXML2_CFLAGS) \ $(NULL) nbdinfo_LDADD = \ $(top_builddir)/lib/libnbd.la \ - $(LIBXML2_LIBS) \ $(NULL) if HAVE_POD @@ -59,6 +55,8 @@ nbdinfo.1: nbdinfo.pod $(top_builddir)/podwrapper.pl endif HAVE_POD +if HAVE_LIBXML2 + TESTS += \ info-list.sh \ info-list-json.sh \ diff --git a/info/nbdinfo.c b/info/nbdinfo.c index b54dfd4..394f5ac 100644 --- a/info/nbdinfo.c +++ b/info/nbdinfo.c @@ -29,7 +29,6 @@ #include <limits.h> #include <errno.h> -#include <libxml/uri.h> #include <libnbd.h> static bool list_all = false; @@ -422,7 +421,6 @@ static void list_all_exports (struct nbd_handle *nbd1, const char *uri) { int i; - xmlURIPtr xmluri = NULL; int count = nbd_get_nr_list_exports (nbd1); if (count == -1) { @@ -434,49 +432,38 @@ list_all_exports (struct nbd_handle *nbd1, const char *uri) printf ("\t\"exports\": []\n"); for (i = 0; i < count; ++i) { - char *name, *desc, *new_path, *new_uri; + char *name, *desc; struct nbd_handle *nbd2; name = nbd_get_list_export_name (nbd1, i); - if (name) { - /* We have to modify the original URI to change the export name. - * In the URI spec, paths always start with '/' (which is ignored). - */ - xmluri = xmlParseURI (uri); - if (!xmluri) { - fprintf (stderr, "unable to parse original URI: %s\n", uri); - exit (EXIT_FAILURE); - } - if (asprintf (&new_path, "/%s", name) == -1) { - perror ("asprintf"); - exit (EXIT_FAILURE); - } - free (xmluri->path); - xmluri->path = new_path; - new_uri = (char *) xmlSaveUri (xmluri); + if (!name) { + fprintf (stderr, "unable to obtain export name: %s\n", + nbd_get_error ()); + exit (EXIT_FAILURE); + } - /* Connect to the new URI. */ - nbd2 = nbd_create (); - if (nbd2 == NULL) { - fprintf (stderr, "%s\n", nbd_get_error ()); - exit (EXIT_FAILURE); - } - nbd_set_uri_allow_local_file (nbd2, true); /* Allow ?tls-psk-file. */ + /* Connect to the original URI, but using opt mode to alter the export. */ + nbd2 = nbd_create (); + if (nbd2 == NULL) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + nbd_set_uri_allow_local_file (nbd2, true); /* Allow ?tls-psk-file. */ + nbd_set_opt_mode (nbd2, true); - if (nbd_connect_uri (nbd2, new_uri) == -1) { - fprintf (stderr, "%s\n", nbd_get_error ()); - exit (EXIT_FAILURE); - } + if (nbd_connect_uri (nbd2, uri) == -1 || + nbd_set_export_name (nbd2, name) == -1 || + nbd_opt_go (nbd2) == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } - /* List the metadata of this export. */ - desc = nbd_get_list_export_description (nbd1, i); - list_one_export (nbd2, desc, i == 0, i + 1 == count); + /* List the metadata of this export. */ + desc = nbd_get_list_export_description (nbd1, i); + list_one_export (nbd2, desc, i == 0, i + 1 == count); - nbd_close (nbd2); - free (new_uri); - free (desc); - xmlFreeURI (xmluri); /* this also frees xmluri->path == new_path */ - } + nbd_close (nbd2); + free (desc); free (name); } } -- 2.28.0

Adding NBD_OPT_LIST support will occur across two commits. This patch switches the existing nbd_set_list_exports() (called at a distance compared to where its results are used) to instead be the synchronous nbd_opt_list() command; the next patch will then focus on making the API friendlier to async usage by shifting from libnbd malloc'ing the results to instead having the caller supply a callback function. Thus, the API signature in this patch is not what we intend to be final for 1.4, but rather a division to make review of the transformation easier. --- lib/internal.h | 3 +- generator/API.ml | 84 ++++++++++-------------- generator/state_machine.ml | 4 +- generator/states-newstyle-opt-list.c | 12 ++-- generator/states-newstyle-opt-starttls.c | 8 +-- generator/states-newstyle.c | 3 + lib/handle.c | 26 ++------ lib/opt.c | 33 ++++++++++ tests/newstyle-limited.c | 9 ++- examples/list-exports.c | 22 ++++--- interop/list-exports.c | 11 ++-- info/nbdinfo.c | 33 ++++------ 12 files changed, 131 insertions(+), 117 deletions(-) diff --git a/lib/internal.h b/lib/internal.h index 3672538..3d059cc 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -103,8 +103,7 @@ struct nbd_handle { bool opt_mode; uint8_t current_opt; - /* List exports mode. */ - bool list_exports; + /* Results of nbd_opt_list. */ size_t nr_exports; struct export *exports; diff --git a/generator/API.ml b/generator/API.ml index 21485ea..c660960 100644 --- a/generator/API.ml +++ b/generator/API.ml @@ -288,12 +288,14 @@ The encoding of export names is always UTF-8. When option mode is not in use, the export name must be set before beginning a connection. However, when L<nbd_set_opt_mode(3)> has enabled option mode, it is possible to change the export -name prior to L<nbd_opt_go(3)>. +name prior to L<nbd_opt_go(3)>. In particular, the use of +L<nbd_opt_list(3)> during negotiation can be used to determine +a name the server is likely to accept. This call may be skipped if using L<nbd_connect_uri(3)> to connect to a URI that includes an export name."; see_also = [Link "get_export_name"; Link "connect_uri"; - Link "set_opt_mode"; Link "opt_go"]; + Link "set_opt_mode"; Link "opt_go"; Link "opt_list"]; }; "get_export_name", { @@ -709,7 +711,7 @@ export name before trying again. You may also use L<nbd_opt_abort(3)> to end the connection without finishing negotiation."; example = Some "examples/list-exports.c"; see_also = [Link "get_opt_mode"; Link "aio_is_negotiating"; - Link "opt_abort"; Link "opt_go"]; + Link "opt_abort"; Link "opt_go"; Link "opt_list"]; }; "get_opt_mode", { @@ -753,68 +755,55 @@ enabled option mode."; see_also = [Link "set_opt_mode"; Link "aio_opt_abort"; Link "opt_go"]; }; - "set_list_exports", { + "opt_list", { default_call with - args = [Bool "list"]; ret = RErr; - permitted_states = [ Created ]; - shortdesc = "set whether to list server exports"; + args = []; ret = RErr; + permitted_states = [ Negotiating ]; + shortdesc = "request the server to list all exports during negotiation"; longdesc = "\ -Set this flag to true in order to list NBD exports provided -by the server. +Request that the server list all exports that it supports. This can +only be used if L<nbd_set_opt_mode(3)> enabled option mode. In this mode, during connection we query the server for the list of NBD exports and collect them in the handle. You can query the list of exports provided by the server by calling L<nbd_get_nr_list_exports(3)> and L<nbd_get_list_export_name(3)>. -After choosing the export you want, you should close this handle, -create a new NBD handle (L<nbd_create(3)>), set the export name -(L<nbd_set_export_name(3)>), and connect on the new handle -(C<nbd_connect_*>). +After choosing the export you want, set the export name +(L<nbd_set_export_name(3)>), then finish connecting with L<nbd_opt_go(3)>. Some servers do not support listing exports at all. In that case the connect call will fail with errno C<ENOTSUP> and L<nbd_get_nr_list_exports(3)> will return 0. -Some servers do not respond with all the exports they -support, either because of an incomplete implementation of -this feature, or because they only list exports relevant -to non-TLS or TLS when a corresponding non-TLS or TLS -connection is opened. - -For L<qemu-nbd(8)> when using the I<-x> option you may find -that the original connect call fails with -C<server has no export named ''> and errno C<ENOENT>. -However you can still proceed to call L<nbd_get_nr_list_exports(3)> etc. +Not all servers understand this request, and even when it is understood, +the server might intentionally send an empty list to avoid being an +information leak or may encounter a failure after delivering partial +results. Thus, this function may succeed even when no exports +are reported, or may fail but have a non-empty list. Likewise, +the NBD protocol does not specify an upper bound for the number of +exports that might be advertised, so client code should be aware that +a server may send a lengthy list; libnbd truncates the server reply +after 10000 exports. For L<nbd-server(1)> you will need to allow clients to make list requests by adding C<allowlist=true> to the C<[generic]> -section of F</etc/nbd-server/config>."; +section of F</etc/nbd-server/config>. For L<qemu-nbd(8)>, a +description is set with I<-D>."; example = Some "examples/list-exports.c"; - see_also = [Link "get_list_exports"; + see_also = [Link "set_opt_mode"; Link "opt_go"; Link "get_nr_list_exports"; Link "get_list_export_name"]; }; - "get_list_exports", { - default_call with - args = []; ret = RBool; - may_set_error = false; - shortdesc = "return whether list exports mode was enabled"; - longdesc = "\ -Return true if list exports mode was enabled on this handle."; - see_also = [Link "set_list_exports"]; - }; - "get_nr_list_exports", { default_call with args = []; ret = RInt; permitted_states = [ Negotiating; Connected; Closed; Dead ]; shortdesc = "return the number of exports returned by the server"; longdesc = "\ -If list exports mode was enabled on the handle and you connected -to the server, this returns the number of exports returned by the -server. This may be 0 or incomplete for reasons given in -L<nbd_set_list_exports(3)>."; - see_also = [Link "set_list_exports"; Link "get_list_export_name"; +If option mode is in effect and you called L<nbd_opt_list(3)>, +this returns the number of exports returned by the server. This +may be 0 or incomplete for reasons given in L<nbd_opt_list(3)>."; + see_also = [Link "opt_list"; Link "get_list_export_name"; Link "get_list_export_description"]; }; @@ -824,10 +813,10 @@ L<nbd_set_list_exports(3)>."; permitted_states = [ Negotiating; Connected; Closed; Dead ]; shortdesc = "return the i'th export name"; longdesc = "\ -If list exports mode was enabled on the handle and you connected -to the server, this can be used to return the i'th export name +If L<nbd_opt_list(3)> succeeded with option mode enabled, +this can be used to return the i'th export name from the list returned by the server."; - see_also = [Link "set_list_exports"; Link "get_list_export_description"]; + see_also = [Link "opt_list"; Link "get_list_export_description"]; }; "get_list_export_description", { @@ -836,13 +825,13 @@ from the list returned by the server."; permitted_states = [ Negotiating; Connected; Closed; Dead ]; shortdesc = "return the i'th export description"; longdesc = "\ -If list exports mode was enabled on the handle and you connected -to the server, this can be used to return the i'th export description +If L<nbd_opt_list(3)> succeeded with option mode enabled, +this can be used to return the i'th export description from the list returned by the server, which may be an empty string. Many servers omit a description. For L<qemu-nbd(8)>, a description is set with I<-D>."; - see_also = [Link "set_list_exports"; Link "get_list_export_name"]; + see_also = [Link "set_opt_mode"; Link "opt_go"]; }; "add_meta_context", { @@ -2559,8 +2548,6 @@ let first_version = [ "set_uri_allow_local_file", (1, 2); (* Added in 1.3.x development cycle, will be stable and supported in 1.4. *) - "set_list_exports", (1, 4); - "get_list_exports", (1, 4); "get_nr_list_exports", (1, 4); "get_list_export_name", (1, 4); "get_list_export_description", (1, 4); @@ -2574,6 +2561,7 @@ let first_version = [ "aio_is_negotiating", (1, 4); "opt_go", (1, 4); "opt_abort", (1, 4); + "opt_list", (1, 4); "aio_opt_go", (1, 4); "aio_opt_abort", (1, 4); diff --git a/generator/state_machine.ml b/generator/state_machine.ml index 2d28c2e..c1fb073 100644 --- a/generator/state_machine.ml +++ b/generator/state_machine.ml @@ -282,12 +282,14 @@ and newstyle_state_machine = [ * NEGOTIATING after OPT_STRUCTURED_REPLY or any failed OPT_GO. *) Group ("OPT_STARTTLS", newstyle_opt_starttls_state_machine); - Group ("OPT_LIST", newstyle_opt_list_state_machine); Group ("OPT_STRUCTURED_REPLY", newstyle_opt_structured_reply_state_machine); Group ("OPT_SET_META_CONTEXT", newstyle_opt_set_meta_context_state_machine); Group ("OPT_GO", newstyle_opt_go_state_machine); Group ("OPT_EXPORT_NAME", newstyle_opt_export_name_state_machine); + (* Options that can be used during negotiation, when opt_mode is enabled. *) + Group ("OPT_LIST", newstyle_opt_list_state_machine); + (* When NBD_OPT_GO fails, or when opt_mode is enabled, option parsing * can be cleanly ended without moving through the %READY state. *) diff --git a/generator/states-newstyle-opt-list.c b/generator/states-newstyle-opt-list.c index f2846b6..19b353e 100644 --- a/generator/states-newstyle-opt-list.c +++ b/generator/states-newstyle-opt-list.c @@ -18,17 +18,13 @@ /* State machine for sending NBD_OPT_LIST to list exports. * - * This is skipped unless list exports mode was enabled on the handle. + * This is only reached via nbd_opt_list during opt_mode. */ STATE_MACHINE { NEWSTYLE.OPT_LIST.START: assert (h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE); - if (!h->list_exports) { - SET_NEXT_STATE (%^OPT_STRUCTURED_REPLY.START); - return 0; - } - + assert (h->opt_mode && h->exports && !h->nr_exports); h->sbuf.option.version = htobe64 (NBD_NEW_VERSION); h->sbuf.option.option = htobe32 (NBD_OPT_LIST); h->sbuf.option.optlen = 0; @@ -135,7 +131,7 @@ STATE_MACHINE { case NBD_REP_ACK: /* Finished receiving the list. */ - SET_NEXT_STATE (%^OPT_STRUCTURED_REPLY.START); + SET_NEXT_STATE (%.NEGOTIATING); return 0; default: @@ -145,7 +141,7 @@ STATE_MACHINE { } set_error (ENOTSUP, "unexpected response, possibly the server does not " "support listing exports"); - SET_NEXT_STATE (%.DEAD); + SET_NEXT_STATE (%.NEGOTIATING); return 0; } return 0; diff --git a/generator/states-newstyle-opt-starttls.c b/generator/states-newstyle-opt-starttls.c index d107b74..9eab023 100644 --- a/generator/states-newstyle-opt-starttls.c +++ b/generator/states-newstyle-opt-starttls.c @@ -23,7 +23,7 @@ STATE_MACHINE { assert (h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE); /* If TLS was not requested we skip this option and go to the next one. */ if (h->tls == LIBNBD_TLS_DISABLE) { - SET_NEXT_STATE (%^OPT_LIST.START); + SET_NEXT_STATE (%^OPT_STRUCTURED_REPLY.START); return 0; } @@ -102,7 +102,7 @@ STATE_MACHINE { debug (h, "server refused TLS (%s), continuing with unencrypted connection", reply == NBD_REP_ERR_POLICY ? "policy" : "not supported"); - SET_NEXT_STATE (%^OPT_LIST.START); + SET_NEXT_STATE (%^OPT_STRUCTURED_REPLY.START); return 0; } return 0; @@ -121,7 +121,7 @@ STATE_MACHINE { nbd_internal_crypto_debug_tls_enabled (h); /* Continue with option negotiation. */ - SET_NEXT_STATE (%^OPT_LIST.START); + SET_NEXT_STATE (%^OPT_STRUCTURED_REPLY.START); return 0; } /* Continue handshake. */ @@ -144,7 +144,7 @@ STATE_MACHINE { debug (h, "connection is using TLS"); /* Continue with option negotiation. */ - SET_NEXT_STATE (%^OPT_LIST.START); + SET_NEXT_STATE (%^OPT_STRUCTURED_REPLY.START); return 0; } /* Continue handshake. */ diff --git a/generator/states-newstyle.c b/generator/states-newstyle.c index d771097..25ef351 100644 --- a/generator/states-newstyle.c +++ b/generator/states-newstyle.c @@ -124,6 +124,9 @@ STATE_MACHINE { else SET_NEXT_STATE (%OPT_SET_META_CONTEXT.START); return 0; + case NBD_OPT_LIST: + SET_NEXT_STATE (%OPT_LIST.START); + return 0; case NBD_OPT_ABORT: if ((h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE) == 0) { SET_NEXT_STATE (%.DEAD); diff --git a/lib/handle.c b/lib/handle.c index a51b923..c308461 100644 --- a/lib/handle.c +++ b/lib/handle.c @@ -288,25 +288,11 @@ nbd_unlocked_get_export_description (struct nbd_handle *h) return r; } -int -nbd_unlocked_set_list_exports (struct nbd_handle *h, bool list) -{ - h->list_exports = list; - return 0; -} - -/* NB: may_set_error = false. */ -int -nbd_unlocked_get_list_exports (struct nbd_handle *h) -{ - return h->list_exports; -} - int nbd_unlocked_get_nr_list_exports (struct nbd_handle *h) { - if (!h->list_exports) { - set_error (EINVAL, "list exports mode not selected on this handle"); + if (!h->exports) { + set_error (EINVAL, "nbd_opt_list not yet run on this handle"); return -1; } return (int) h->nr_exports; @@ -318,8 +304,8 @@ nbd_unlocked_get_list_export_name (struct nbd_handle *h, { char *name; - if (!h->list_exports) { - set_error (EINVAL, "list exports mode not selected on this handle"); + if (!h->exports) { + set_error (EINVAL, "nbd_opt_list not yet run on this handle"); return NULL; } if (i < 0 || i >= (int) h->nr_exports) { @@ -340,8 +326,8 @@ nbd_unlocked_get_list_export_description (struct nbd_handle *h, { char *desc; - if (!h->list_exports) { - set_error (EINVAL, "list exports mode not selected on this handle"); + if (!h->exports) { + set_error (EINVAL, "nbd_opt_list not yet run on this handle"); return NULL; } if (i < 0 || i >= (int) h->nr_exports) { diff --git a/lib/opt.c b/lib/opt.c index 6ad7f1a..7458b4b 100644 --- a/lib/opt.c +++ b/lib/opt.c @@ -78,6 +78,39 @@ nbd_unlocked_opt_abort (struct nbd_handle *h) return wait_for_option (h); } +/* Issue NBD_OPT_LIST and wait for the reply. */ +int +nbd_unlocked_opt_list (struct nbd_handle *h) +{ + size_t i; + + if ((h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE) == 0) { + set_error (ENOTSUP, "server is not using fixed newstyle protocol"); + return -1; + } + + /* Overwrite any previous results */ + if (h->exports) { + for (i = 0; i < h->nr_exports; ++i) { + free (h->exports[i].name); + free (h->exports[i].description); + } + h->nr_exports = 0; + } + else { + h->exports = malloc (sizeof *h->exports); + if (h->exports == NULL) { + set_error (errno, "malloc"); + return -1; + } + } + + h->current_opt = NBD_OPT_LIST; + if (nbd_internal_run (h, cmd_issue) == -1) + debug (h, "option queued, ignoring state machine failure"); + return wait_for_option (h); +} + /* Issue NBD_OPT_GO (or NBD_OPT_EXPORT_NAME) without waiting. */ int nbd_unlocked_aio_opt_go (struct nbd_handle *h) diff --git a/tests/newstyle-limited.c b/tests/newstyle-limited.c index aa6f87b..f2f1cba 100644 --- a/tests/newstyle-limited.c +++ b/tests/newstyle-limited.c @@ -136,7 +136,14 @@ main (int argc, char *argv[]) fprintf (stderr, "unexpected state after negotiating\n"); exit (EXIT_FAILURE); } - /* XXX Test nbd_opt_list when it is implemented */ + if (nbd_opt_list (nbd) != -1) { + fprintf (stderr, "nbd_opt_list: expected failure\n"); + exit (EXIT_FAILURE); + } + if (nbd_get_errno () != ENOTSUP) { + fprintf (stderr, "%s: wrong errno value after failed opt_list\n", argv[0]); + exit (EXIT_FAILURE); + } if (nbd_opt_abort (nbd) == -1) { fprintf (stderr, "%s\n", nbd_get_error ()); exit (EXIT_FAILURE); diff --git a/examples/list-exports.c b/examples/list-exports.c index daea2ab..fcd474b 100644 --- a/examples/list-exports.c +++ b/examples/list-exports.c @@ -55,29 +55,31 @@ main (int argc, char *argv[]) exit (EXIT_FAILURE); } - /* Set opt mode and request list exports. */ + /* Set opt mode. */ nbd_set_opt_mode (nbd, true); - nbd_set_list_exports (nbd, true); /* Connect to the NBD server over a - * Unix domain socket. A side effect of - * connecting is to list the exports. - * This operation can fail normally, so - * we need to check the return value and - * error code. + * Unix domain socket. If we did not + * end up in option mode, then a + * listing is not possible. */ r = nbd_connect_unix (nbd, argv[1]); - if (r == -1 && nbd_get_errno () == ENOTSUP) { + if (r == -1) { fprintf (stderr, "%s\n", nbd_get_error ()); exit (EXIT_FAILURE); } - if (!nbd_aio_is_negotiating (nbd) || - nbd_get_nr_list_exports (nbd) == 0) { + if (!nbd_aio_is_negotiating (nbd)) { fprintf (stderr, "Server does not support " "listing exports.\n"); exit (EXIT_FAILURE); } + /* Grab the export list. */ + if (nbd_opt_list (nbd) == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + /* Display the list of exports. */ for (i = 0; i < nbd_get_nr_list_exports (nbd); diff --git a/interop/list-exports.c b/interop/list-exports.c index d003ce9..5af1234 100644 --- a/interop/list-exports.c +++ b/interop/list-exports.c @@ -52,8 +52,8 @@ main (int argc, char *argv[]) exit (EXIT_FAILURE); } - /* Set the list exports mode in the handle. */ - nbd_set_list_exports (nbd, true); + /* Set option mode in the handle. */ + nbd_set_opt_mode (nbd, true); /* Run qemu-nbd. */ char *args[] = { SERVER, SERVER_PARAMS, NULL }; @@ -62,9 +62,11 @@ main (int argc, char *argv[]) #else #define NBD_CONNECT nbd_connect_command #endif - if (NBD_CONNECT (nbd, args) == -1) - /* This is not an error so don't fail here. */ + if (NBD_CONNECT (nbd, args) == -1 || nbd_opt_list (nbd) == -1) { fprintf (stderr, "%s\n", nbd_get_error ()); + unlink (tmpfile); + exit (EXIT_FAILURE); + } /* We don't need the temporary file any longer. */ unlink (tmpfile); @@ -98,6 +100,7 @@ main (int argc, char *argv[]) free (desc); } + nbd_opt_abort (nbd); nbd_close (nbd); exit (EXIT_SUCCESS); } diff --git a/info/nbdinfo.c b/info/nbdinfo.c index 394f5ac..cdc0db8 100644 --- a/info/nbdinfo.c +++ b/info/nbdinfo.c @@ -184,21 +184,27 @@ main (int argc, char *argv[]) } nbd_set_uri_allow_local_file (nbd, true); /* Allow ?tls-psk-file. */ - /* If using --list then we set the list_exports mode flag in the - * handle. In this case it can be OK for the connection to fail. - * In particular it can fail because the export in the URI is not - * recognized. - */ + /* If using --list then we need opt mode in the handle. */ if (list_all) - nbd_set_list_exports (nbd, true); + nbd_set_opt_mode (nbd, true); else if (!size_only) nbd_set_full_info (nbd, true); if (nbd_connect_uri (nbd, argv[optind]) == -1) { - if (!list_all || nbd_get_errno () == ENOTSUP) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + + if (list_all) { + if (nbd_opt_list (nbd) == -1) { fprintf (stderr, "%s\n", nbd_get_error ()); exit (EXIT_FAILURE); } + /* Disconnect from the server to move the handle into a closed + * state, in case the server serializes further connections. + * But we can ignore errors in this case. + */ + nbd_opt_abort (nbd); } if (size_only) { @@ -211,21 +217,10 @@ main (int argc, char *argv[]) printf ("%" PRIi64 "\n", size); } else { - /* Print per-connection fields. - * - * XXX Not always displayed when using --list mode. This is - * because if the connection fails above (as we expect) then the - * handle state is dead and we cannot query these. - */ + /* Print per-connection fields. */ protocol = nbd_get_protocol (nbd); tls_negotiated = nbd_get_tls_negotiated (nbd); - /* Disconnect from the server to move the handle into a closed - * state, in case the server serializes further connections; but - * ignore errors as the connection may already be dead. - */ - nbd_shutdown (nbd, 0); - if (!json_output) { if (protocol) { printf ("protocol: %s", protocol); -- 2.28.0

Squash this into opt_go? into opt_info? Standalone? Testing: why does python not throw an exception: $ ./run nbdsh Welcome to nbdsh, the shell for interacting with Network Block Device (NBD) servers. The ‘nbd’ module has already been imported and there is an open NBD handle called ‘h’. h.connect_tcp ("remote", "10809") # Connect to a remote server. h.get_size () # Get size of the remote disk. buf = h.pread (512, 0, 0) # Read the first sector. exit() or Ctrl-D # Quit the shell help (nbd) # Display documentation nbd> h.set_opt_mode(True) nbd> h.connect_command(["nbdkit","-s","--exit-","eval", "open=echo ENOENT >&2; exit 1"]) nbd> h.aio_is_negotiating() True nbd> h.set_export_name('a') nbd> h.opt_go() nbdkit: eval: error: /tmp/nbdkitNyQD8J/open: ENOENT nbd> h.set_export_name('b') nbd> h.opt_go() nbdkit: eval: error: /tmp/nbdkitNyQD8J/open: ENOENT nbd> h.set_opt_mode(True) Traceback (most recent call last): File "/usr/lib64/python3.8/code.py", line 90, in runcode exec(code, self.locals) File "<console>", line 1, in <module> File "/home/eblake/libnbd/python/nbd.py", line 570, in set_opt_mode return libnbdmod.set_opt_mode (self._o, enable) nbd.Error: nbd_set_opt_mode: invalid state: NEGOTIATING: the handle must be newly created: Invalid argument (EINVAL) nbd> quit() --- generator/API.ml | 12 ++++++++++-- generator/states-newstyle-opt-go.c | 19 +++++++++++++------ lib/opt.c | 18 ++++++++++++++++-- 3 files changed, 39 insertions(+), 10 deletions(-) diff --git a/generator/API.ml b/generator/API.ml index 52970d3..bf50030 100644 --- a/generator/API.ml +++ b/generator/API.ml @@ -1876,7 +1876,9 @@ on the connection."; "aio_opt_go", { default_call with - args = []; ret = RErr; + args = []; + optargs = [ OClosure completion_closure ]; + ret = RErr; permitted_states = [ Negotiating ]; shortdesc = "end negotiation and move on to using an export"; longdesc = "\ @@ -1885,7 +1887,13 @@ export previously specified by L<nbd_set_export_name(3)>. This can only be used if L<nbd_set_opt_mode(3)> enabled option mode. To determine when the request completes, wait for -L<nbd_aio_is_connecting(3)> to return false."; +L<nbd_aio_is_connecting(3)> to return false. Or supply the optional +C<completion_callback> which will be invoked as described in +L<libnbd(3)/Completion callbacks>, except that it is automatically +retired regardless of return value. Note that detecting whether the +server returns an error (as is done by the return value of the +synchronous counterpart) is only possible with a completion +callback."; see_also = [Link "set_opt_mode"; Link "opt_go"]; }; diff --git a/generator/states-newstyle-opt-go.c b/generator/states-newstyle-opt-go.c index 0568695..95cc041 100644 --- a/generator/states-newstyle-opt-go.c +++ b/generator/states-newstyle-opt-go.c @@ -124,6 +124,7 @@ STATE_MACHINE { uint64_t exportsize; uint16_t eflags; uint32_t min, pref, max; + int err; reply = be32toh (h->sbuf.or.option_reply.reply); len = be32toh (h->sbuf.or.option_reply.replylen); @@ -241,15 +242,21 @@ STATE_MACHINE { reply); } nbd_internal_reset_size_and_flags (h); - if (h->opt_mode) - SET_NEXT_STATE (%.NEGOTIATING); - else - SET_NEXT_STATE (%^PREPARE_OPT_ABORT); + err = nbd_get_errno (); break; case NBD_REP_ACK: + err = 0; + break; + } + + CALL_CALLBACK (h->opt_cb.completion, &err); + nbd_internal_free_option (h); + if (err == 0) SET_NEXT_STATE (%^FINISHED); - break; - } + else if (h->opt_mode) + SET_NEXT_STATE (%.NEGOTIATING); + else + SET_NEXT_STATE (%^PREPARE_OPT_ABORT); return 0; } /* END STATE MACHINE */ diff --git a/lib/opt.c b/lib/opt.c index 1a5f645..17f2508 100644 --- a/lib/opt.c +++ b/lib/opt.c @@ -60,16 +60,28 @@ wait_for_option (struct nbd_handle *h) return 0; } +static int +go_complete (void *opaque, int *err) +{ + int *i = opaque; + *i = *err; + return 0; +} + /* Issue NBD_OPT_GO (or NBD_OPT_EXPORT_NAME) and wait for the reply. */ int nbd_unlocked_opt_go (struct nbd_handle *h) { - int r = nbd_unlocked_aio_opt_go (h); + int err; + nbd_completion_callback c = { .callback = go_complete, .user_data = &err }; + int r = nbd_unlocked_aio_opt_go (h, c); if (r == -1) return r; r = wait_for_option (h); + if (r == 0) + r = err; if (r == 0 && nbd_internal_is_state_negotiating (get_next_state (h))) return -1; /* NBD_OPT_GO failed, but can be attempted again */ return r; @@ -132,9 +144,11 @@ nbd_unlocked_opt_list (struct nbd_handle *h, nbd_list_callback list) /* Issue NBD_OPT_GO (or NBD_OPT_EXPORT_NAME) without waiting. */ int -nbd_unlocked_aio_opt_go (struct nbd_handle *h) +nbd_unlocked_aio_opt_go (struct nbd_handle *h, + nbd_completion_callback complete) { h->current_opt = NBD_OPT_GO; + h->opt_cb.completion = complete; if (nbd_internal_run (h, cmd_issue) == -1) debug (h, "option queued, ignoring state machine failure"); -- 2.28.0

Make it possible to query details about a potential export without actually connecting. This resues the NBD_OPT_GO state machine, with the main difference being that we can now return to negotiating state even on success; thus checking for progression to Ready state is no longer sufficient for detecting failure, and we have to rely on the completion callback. And now that we can get to Negotiation after a successful query, we have to enable a lot more functions for use in that state to read the results of the info query. The next patch will then update nbdinfo to take advantage of this. [TODO: fix tests/opt-info to actually test what is needed...] --- docs/libnbd.pod | 4 ++ generator/API.ml | 108 +++++++++++++++++++++-------- generator/states-newstyle-opt-go.c | 29 +++++--- lib/opt.c | 35 ++++++++++ tests/Makefile.am | 7 ++ tests/newstyle-limited.c | 8 +++ tests/opt-info.c | 107 ++++++++++++++++++++++++++++ .gitignore | 1 + 8 files changed, 262 insertions(+), 37 deletions(-) create mode 100644 tests/opt-info.c diff --git a/docs/libnbd.pod b/docs/libnbd.pod index 4ee0815..9cdf152 100644 --- a/docs/libnbd.pod +++ b/docs/libnbd.pod @@ -493,6 +493,10 @@ Note that there are some servers (like L<nbdkit(1)> E<le> 1.14) which ignore this, and other servers (like L<qemu-nbd(8)>) which require it to be set correctly but cannot serve different content. +These APIs are also available after a successful L<nbd_opt_info(3)> +during the negotiation phase, if you used L<nbd_set_opt_mode(3)> prior +to connecting. + =head2 Flag calls After connecting the server will send back a set of flags describing diff --git a/generator/API.ml b/generator/API.ml index bf50030..e703de7 100644 --- a/generator/API.ml +++ b/generator/API.ml @@ -294,12 +294,14 @@ before beginning a connection. However, when L<nbd_set_opt_mode(3)> has enabled option mode, it is possible to change the export name prior to L<nbd_opt_go(3)>. In particular, the use of L<nbd_opt_list(3)> during negotiation can be used to determine -a name the server is likely to accept. +a name the server is likely to accept, and L<nbd_opt_info(3)> can +be used to learn details about an export before connecting. This call may be skipped if using L<nbd_connect_uri(3)> to connect to a URI that includes an export name."; see_also = [Link "get_export_name"; Link "connect_uri"; - Link "set_opt_mode"; Link "opt_go"; Link "opt_list"]; + Link "set_opt_mode"; Link "opt_go"; Link "opt_list"; + Link "opt_info"]; }; "get_export_name", { @@ -353,7 +355,7 @@ Return the state of the full info request flag on this handle."; "get_canonical_export_name", { default_call with args = []; ret = RString; - permitted_states = [ Connected; Closed ]; + permitted_states = [ Negotiating; Connected; Closed ]; shortdesc = "return the canonical export name, if the server has one"; longdesc = "\ The NBD protocol permits a server to report an optional canonical @@ -373,7 +375,7 @@ client specifically hinted about wanting it, via L<nbd_set_full_info(3)>."; "get_export_description", { default_call with args = []; ret = RString; - permitted_states = [ Connected; Closed ]; + permitted_states = [ Negotiating; Connected; Closed ]; shortdesc = "return the export description, if the server has one"; longdesc = "\ The NBD protocol permits a server to report an optional export @@ -715,7 +717,8 @@ export name before trying again. You may also use L<nbd_opt_abort(3)> to end the connection without finishing negotiation."; example = Some "examples/list-exports.c"; see_also = [Link "get_opt_mode"; Link "aio_is_negotiating"; - Link "opt_abort"; Link "opt_go"; Link "opt_list"]; + Link "opt_abort"; Link "opt_go"; Link "opt_list"; + Link "opt_info"]; }; "get_opt_mode", { @@ -743,7 +746,7 @@ possible to attempt another option such as a different export name; although older servers will instead have killed the connection."; example = Some "examples/list-exports.c"; see_also = [Link "set_opt_mode"; Link "aio_opt_go"; Link "opt_abort"; - Link "set_export_name"]; + Link "set_export_name"; Link "opt_info"]; }; "opt_abort", { @@ -798,6 +801,29 @@ description is set with I<-D>."; Link "set_export_name"]; }; + "opt_info", { + default_call with + args = []; ret = RErr; + permitted_states = [ Negotiating ]; + shortdesc = "request the server for information about an export"; + longdesc = "\ +Request that the server supply information about the most recent +export name set by L<nbd_set_export_name(3)>. This can +only be used if L<nbd_set_opt_mode(3)> enabled option mode. + +If successful, functions like L<nbd_is_read_only(3)> and +L<nbd_get_size(3)> will report details about that export. In +general, if L<nbd_opt_go(3)> is called next, that call will +likely succeed with the details remaining the same, although this +is not guaranteed by all servers. + +Not all servers understand this request, and even when it is +understood, the server might fail the request even when a +corresponding L<nbd_opt_go(3)> would succeed."; + see_also = [Link "set_opt_mode"; Link "aio_opt_info"; Link "opt_go"; + Link "set_export_name"]; + }; + "add_meta_context", { default_call with args = [ String "name" ]; ret = RErr; @@ -1180,27 +1206,27 @@ is killed."; "is_read_only", { default_call with args = []; ret = RBool; - permitted_states = [ Connected; Closed ]; + permitted_states = [ Negotiating; Connected; Closed ]; shortdesc = "is the NBD export read-only?"; longdesc = "\ Returns true if the NBD export is read-only; writes and write-like operations will fail." ^ non_blocking_test_call_description; - see_also = [SectionLink "Flag calls"]; + see_also = [SectionLink "Flag calls"; Link "opt_info"]; example = Some "examples/server-flags.c"; }; "can_flush", { default_call with args = []; ret = RBool; - permitted_states = [ Connected; Closed ]; + permitted_states = [ Negotiating; Connected; Closed ]; shortdesc = "does the server support the flush command?"; longdesc = "\ Returns true if the server supports the flush command (see L<nbd_flush(3)>, L<nbd_aio_flush(3)>). Returns false if the server does not." ^ non_blocking_test_call_description; - see_also = [SectionLink "Flag calls"; + see_also = [SectionLink "Flag calls"; Link "opt_info"; Link "flush"; Link "aio_flush"]; example = Some "examples/server-flags.c"; }; @@ -1208,13 +1234,13 @@ the server does not." "can_fua", { default_call with args = []; ret = RBool; - permitted_states = [ Connected; Closed ]; + permitted_states = [ Negotiating; Connected; Closed ]; shortdesc = "does the server support the FUA flag?"; longdesc = "\ Returns true if the server supports the FUA flag on certain commands (see L<nbd_pwrite(3)>)." ^ non_blocking_test_call_description; - see_also = [SectionLink "Flag calls"; Link "pwrite"; + see_also = [SectionLink "Flag calls"; Link "opt_info"; Link "pwrite"; Link "zero"; Link "trim"]; example = Some "examples/server-flags.c"; }; @@ -1222,7 +1248,7 @@ certain commands (see L<nbd_pwrite(3)>)." "is_rotational", { default_call with args = []; ret = RBool; - permitted_states = [ Connected; Closed ]; + permitted_states = [ Negotiating; Connected; Closed ]; shortdesc = "is the NBD disk rotational (like a disk)?"; longdesc = "\ Returns true if the disk exposed over NBD is rotational @@ -1230,21 +1256,21 @@ Returns true if the disk exposed over NBD is rotational the disk has no penalty for random access (like an SSD or RAM disk)." ^ non_blocking_test_call_description; - see_also = [SectionLink "Flag calls"]; + see_also = [SectionLink "Flag calls"; Link "opt_info"]; example = Some "examples/server-flags.c"; }; "can_trim", { default_call with args = []; ret = RBool; - permitted_states = [ Connected; Closed ]; + permitted_states = [ Negotiating; Connected; Closed ]; shortdesc = "does the server support the trim command?"; longdesc = "\ Returns true if the server supports the trim command (see L<nbd_trim(3)>, L<nbd_aio_trim(3)>). Returns false if the server does not." ^ non_blocking_test_call_description; - see_also = [SectionLink "Flag calls"; + see_also = [SectionLink "Flag calls"; Link "opt_info"; Link "trim"; Link "aio_trim"]; example = Some "examples/server-flags.c"; }; @@ -1252,14 +1278,14 @@ the server does not." "can_zero", { default_call with args = []; ret = RBool; - permitted_states = [ Connected; Closed ]; + permitted_states = [ Negotiating; Connected; Closed ]; shortdesc = "does the server support the zero command?"; longdesc = "\ Returns true if the server supports the zero command (see L<nbd_zero(3)>, L<nbd_aio_zero(3)>). Returns false if the server does not." ^ non_blocking_test_call_description; - see_also = [SectionLink "Flag calls"; + see_also = [SectionLink "Flag calls"; Link "opt_info"; Link "zero"; Link "aio_zero"; Link "can_fast_zero"]; example = Some "examples/server-flags.c"; @@ -1268,7 +1294,7 @@ the server does not." "can_fast_zero", { default_call with args = []; ret = RBool; - permitted_states = [ Connected; Closed ]; + permitted_states = [ Negotiating; Connected; Closed ]; shortdesc = "does the server support the fast zero flag?"; longdesc = "\ Returns true if the server supports the use of the @@ -1276,7 +1302,7 @@ C<LIBNBD_CMD_FLAG_FAST_ZERO> flag to the zero command (see L<nbd_zero(3)>, L<nbd_aio_zero(3)>). Returns false if the server does not." ^ non_blocking_test_call_description; - see_also = [SectionLink "Flag calls"; + see_also = [SectionLink "Flag calls"; Link "opt_info"; Link "zero"; Link "aio_zero"; Link "can_zero"]; example = Some "examples/server-flags.c"; }; @@ -1284,7 +1310,7 @@ the server does not." "can_df", { default_call with args = []; ret = RBool; - permitted_states = [ Connected; Closed ]; + permitted_states = [ Negotiating; Connected; Closed ]; shortdesc = "does the server support the don't fragment flag to pread?"; longdesc = "\ Returns true if the server supports structured reads with an @@ -1292,7 +1318,7 @@ ability to request a non-fragmented read (see L<nbd_pread_structured(3)>, L<nbd_aio_pread_structured(3)>). Returns false if the server either lacks structured reads or if it does not support a non-fragmented read request." ^ non_blocking_test_call_description; - see_also = [SectionLink "Flag calls"; + see_also = [SectionLink "Flag calls"; Link "opt_info"; Link "pread_structured"; Link "aio_pread_structured"]; example = Some "examples/server-flags.c"; @@ -1301,7 +1327,7 @@ structured reads or if it does not support a non-fragmented read request." "can_multi_conn", { default_call with args = []; ret = RBool; - permitted_states = [ Connected; Closed ]; + permitted_states = [ Negotiating; Connected; Closed ]; shortdesc = "does the server support multi-conn?"; longdesc = "\ Returns true if the server supports multi-conn. Returns @@ -1314,21 +1340,22 @@ way to check for this is to open one connection, check this flag is true, then open further connections as required." ^ non_blocking_test_call_description; - see_also = [SectionLink "Multi-conn"]; + see_also = [SectionLink "Flag calls"; SectionLink "Multi-conn"; + Link "opt_info"]; example = Some "examples/server-flags.c"; }; "can_cache", { default_call with args = []; ret = RBool; - permitted_states = [ Connected; Closed ]; + permitted_states = [ Negotiating; Connected; Closed ]; shortdesc = "does the server support the cache command?"; longdesc = "\ Returns true if the server supports the cache command (see L<nbd_cache(3)>, L<nbd_aio_cache(3)>). Returns false if the server does not." ^ non_blocking_test_call_description; - see_also = [SectionLink "Flag calls"; + see_also = [SectionLink "Flag calls"; Link "opt_info"; Link "cache"; Link "aio_cache"]; example = Some "examples/server-flags.c"; }; @@ -1336,7 +1363,7 @@ the server does not." "can_meta_context", { default_call with args = [String "metacontext"]; ret = RBool; - permitted_states = [ Connected; Closed ]; + permitted_states = [ Negotiating; Connected; Closed ]; shortdesc = "does the server support a specific meta context?"; longdesc = "\ Returns true if the server supports the given meta context @@ -1349,7 +1376,7 @@ B<E<lt>libnbd.hE<gt>> includes defined constants for well-known namespace contexts beginning with C<LIBNBD_CONTEXT_>, but you are free to pass in other contexts." ^ non_blocking_test_call_description; - see_also = [SectionLink "Flag calls"; + see_also = [SectionLink "Flag calls"; Link "opt_info"; Link "add_meta_context"; Link "block_status"; Link "aio_block_status"]; }; @@ -1934,6 +1961,29 @@ callback."; see_also = [Link "set_opt_mode"; Link "opt_list"]; }; + "aio_opt_info", { + default_call with + args = []; + optargs = [ OClosure completion_closure ]; + ret = RErr; + permitted_states = [ Negotiating ]; + shortdesc = "request the server for information about an export"; + longdesc = "\ +Request that the server supply information about the most recent +export name set by L<nbd_set_export_name(3)>. This can +only be used if L<nbd_set_opt_mode(3)> enabled option mode. + +To determine when the request completes, wait for +L<nbd_aio_is_connecting(3)> to return false. Or supply the optional +C<completion_callback> which will be invoked as described in +L<libnbd(3)/Completion callbacks>. Note that direct detection of +whether the server returns an error (as is done by the return value +of the synchronous counterpart) is only possible with a completion +callback; however, indirect detection of failure is possible by +checking whether L<nbd_is_read_only(3)> and similar also fail."; + see_also = [Link "set_opt_mode"; Link "opt_info"; Link "is_read_only"]; + }; + "aio_pread", { default_call with args = [ BytesPersistOut ("buf", "count"); UInt64 "offset" ]; @@ -2553,9 +2603,11 @@ let first_version = [ "opt_go", (1, 4); "opt_abort", (1, 4); "opt_list", (1, 4); + "opt_info", (1, 4); "aio_opt_go", (1, 4); "aio_opt_abort", (1, 4); "aio_opt_list", (1, 4); + "aio_opt_info", (1, 4); (* These calls are proposed for a future version of libnbd, but * have not been added to any released version so far. diff --git a/generator/states-newstyle-opt-go.c b/generator/states-newstyle-opt-go.c index 95cc041..54e83ab 100644 --- a/generator/states-newstyle-opt-go.c +++ b/generator/states-newstyle-opt-go.c @@ -23,8 +23,12 @@ STATE_MACHINE { uint16_t nrinfos = h->full_info ? 3 : 1; assert (h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE); + if (h->current_opt == NBD_OPT_INFO) + assert (h->opt_mode); + else + assert (h->current_opt == NBD_OPT_GO); h->sbuf.option.version = htobe64 (NBD_NEW_VERSION); - h->sbuf.option.option = htobe32 (NBD_OPT_GO); + h->sbuf.option.option = htobe32 (h->current_opt); h->sbuf.option.optlen = htobe32 (/* exportnamelen */ 4 + strlen (h->export_name) + sizeof nrinfos + 2 * nrinfos); @@ -101,7 +105,7 @@ STATE_MACHINE { switch (recv_into_rbuf (h)) { case -1: SET_NEXT_STATE (%.DEAD); return 0; case 0: - if (prepare_for_reply_payload (h, NBD_OPT_GO) == -1) { + if (prepare_for_reply_payload (h, h->current_opt) == -1) { SET_NEXT_STATE (%.DEAD); return 0; } @@ -206,9 +210,12 @@ STATE_MACHINE { SET_NEXT_STATE (%RECV_REPLY); return 0; case NBD_REP_ERR_UNSUP: - debug (h, "server is confused by NBD_OPT_GO, continuing anyway"); - SET_NEXT_STATE (%^OPT_EXPORT_NAME.START); - return 0; + if (h->current_opt == NBD_OPT_GO) { + debug (h, "server is confused by NBD_OPT_GO, continuing anyway"); + SET_NEXT_STATE (%^OPT_EXPORT_NAME.START); + return 0; + } + /* fallthrough */ default: if (handle_reply_error (h) == -1) { SET_NEXT_STATE (%.DEAD); @@ -216,6 +223,10 @@ STATE_MACHINE { } /* Decode expected known errors into a nicer string */ switch (reply) { + case NBD_REP_ERR_UNSUP: + assert (h->current_opt == NBD_OPT_INFO); + set_error (ENOTSUP, "handshake: server lacks NBD_OPT_INFO support"); + break; case NBD_REP_ERR_POLICY: case NBD_REP_ERR_PLATFORM: set_error (0, "handshake: server policy prevents NBD_OPT_GO"); @@ -242,21 +253,21 @@ STATE_MACHINE { reply); } nbd_internal_reset_size_and_flags (h); - err = nbd_get_errno (); + err = nbd_get_errno () ? : ENOTSUP; break; case NBD_REP_ACK: err = 0; break; } - CALL_CALLBACK (h->opt_cb.completion, &err); - nbd_internal_free_option (h); - if (err == 0) + if (err == 0 && h->current_opt == NBD_OPT_GO) SET_NEXT_STATE (%^FINISHED); else if (h->opt_mode) SET_NEXT_STATE (%.NEGOTIATING); else SET_NEXT_STATE (%^PREPARE_OPT_ABORT); + CALL_CALLBACK (h->opt_cb.completion, &err); + nbd_internal_free_option (h); return 0; } /* END STATE MACHINE */ diff --git a/lib/opt.c b/lib/opt.c index 17f2508..d0b9f78 100644 --- a/lib/opt.c +++ b/lib/opt.c @@ -87,6 +87,23 @@ nbd_unlocked_opt_go (struct nbd_handle *h) return r; } +/* Issue NBD_OPT_INFO and wait for the reply. */ +int +nbd_unlocked_opt_info (struct nbd_handle *h) +{ + int err; + nbd_completion_callback c = { .callback = go_complete, .user_data = &err }; + int r = nbd_unlocked_aio_opt_info (h, c); + + if (r == -1) + return r; + + r = wait_for_option (h); + if (r == 0) + r = err; + return r; +} + /* Issue NBD_OPT_ABORT and wait for the state change. */ int nbd_unlocked_opt_abort (struct nbd_handle *h) @@ -155,6 +172,24 @@ nbd_unlocked_aio_opt_go (struct nbd_handle *h, return 0; } +/* Issue NBD_OPT_INFO without waiting. */ +int +nbd_unlocked_aio_opt_info (struct nbd_handle *h, + nbd_completion_callback complete) +{ + if ((h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE) == 0) { + set_error (ENOTSUP, "server is not using fixed newstyle protocol"); + return -1; + } + + h->current_opt = NBD_OPT_INFO; + h->opt_cb.completion = complete; + + if (nbd_internal_run (h, cmd_issue) == -1) + debug (h, "option queued, ignoring state machine failure"); + return 0; +} + /* Issue NBD_OPT_ABORT without waiting. */ int nbd_unlocked_aio_opt_abort (struct nbd_handle *h) diff --git a/tests/Makefile.am b/tests/Makefile.am index 76b370a..81cd30e 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -164,6 +164,7 @@ check_PROGRAMS += \ oldstyle \ newstyle-limited \ opt-abort \ + opt-info \ connect-unix \ connect-tcp \ aio-parallel \ @@ -200,6 +201,7 @@ TESTS += \ oldstyle \ newstyle-limited \ opt-abort \ + opt-info \ connect-unix \ connect-tcp \ aio-parallel.sh \ @@ -380,6 +382,11 @@ opt_abort_CPPFLAGS = -I$(top_srcdir)/include opt_abort_CFLAGS = $(WARNINGS_CFLAGS) opt_abort_LDADD = $(top_builddir)/lib/libnbd.la +opt_info_SOURCES = opt-info.c +opt_info_CPPFLAGS = -I$(top_srcdir)/include +opt_info_CFLAGS = $(WARNINGS_CFLAGS) +opt_info_LDADD = $(top_builddir)/lib/libnbd.la + connect_unix_SOURCES = connect-unix.c connect_unix_CPPFLAGS = -I$(top_srcdir)/include connect_unix_CFLAGS = $(WARNINGS_CFLAGS) diff --git a/tests/newstyle-limited.c b/tests/newstyle-limited.c index ab6c1c2..ce62bdb 100644 --- a/tests/newstyle-limited.c +++ b/tests/newstyle-limited.c @@ -152,6 +152,14 @@ main (int argc, char *argv[]) fprintf (stderr, "%s: wrong errno value after failed opt_list\n", argv[0]); exit (EXIT_FAILURE); } + if (nbd_opt_info (nbd) != -1) { + fprintf (stderr, "nbd_opt_info: expected failure\n"); + exit (EXIT_FAILURE); + } + if (nbd_get_errno () != ENOTSUP) { + fprintf (stderr, "%s: wrong errno value after failed opt_info\n", argv[0]); + exit (EXIT_FAILURE); + } if (nbd_opt_abort (nbd) == -1) { fprintf (stderr, "%s\n", nbd_get_error ()); exit (EXIT_FAILURE); diff --git a/tests/opt-info.c b/tests/opt-info.c new file mode 100644 index 0000000..d237351 --- /dev/null +++ b/tests/opt-info.c @@ -0,0 +1,107 @@ +/* NBD client library in userspace + * Copyright (C) 2013-2020 Red Hat Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library 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 + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +/* Test behavior of nbd_opt_info. */ + +#include <config.h> + +#include <stdio.h> +#include <stdlib.h> +#include <stdint.h> +#include <inttypes.h> +#include <string.h> +#include <errno.h> + +#include <libnbd.h> + +int +main (int argc, char *argv[]) +{ + struct nbd_handle *nbd; + int64_t r; + const char *s; + /* A rather convoluted plugin: export '' is advertised but + * unavailable, export 'a' is unadvertised but available, + * export 'b' is unadvertised and unavailable, and all + * other exports are unadvertised but available. Available + * exports have a size matching the name length. + */ + char *args[] = { "nbdkit", "-s", "--exit-with-parent", "-v", + "memory", "size=512", NULL }; + + /* Quick check that nbdkit is new enough */ + if (system ("nbdkit eval --dump-plugin | grep -q has_list_exports=1")) { + fprintf (stderr, "skipping: nbdkit too old for this test\n"); + exit (77); + } + + /* Get into negotiating state. */ + nbd = nbd_create (); + if (nbd == NULL || + nbd_set_opt_mode (nbd, true) == -1 || + nbd_connect_command (nbd, args) == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + + /* Protocol should be "newstyle-fixed", with structured replies already + * negotiated. + */ + if (nbd_aio_is_negotiating (nbd) != true) { + fprintf (stderr, "unexpected state after connection\n"); + exit (EXIT_FAILURE); + } + s = nbd_get_protocol (nbd); + if (strcmp (s, "newstyle-fixed") != 0) { + fprintf (stderr, + "incorrect protocol \"%s\", expected \"newstyle-fixed\"\n", s); + exit (EXIT_FAILURE); + } + if ((r = nbd_get_structured_replies_negotiated (nbd)) != 1) { + fprintf (stderr, + "incorrect structured replies %" PRId64 ", expected 1\n", r); + exit (EXIT_FAILURE); + } + + /* Quitting negotiation should be graceful. */ + if (nbd_opt_abort (nbd) == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + if (nbd_aio_is_closed (nbd) != true) { + fprintf (stderr, "unexpected state after abort\n"); + exit (EXIT_FAILURE); + } + + /* As negotiation never finished, we have no size. */ + if ((r = nbd_get_size (nbd)) != -1) { + fprintf (stderr, "%s: test failed: incorrect size, " + "actual %" PRIi64 ", expected -1", + argv[0], r); + exit (EXIT_FAILURE); + } + if ((r = nbd_get_errno ()) != EINVAL) { + fprintf (stderr, "%s: test failed: unexpected errno, " + "actual %" PRIi64 ", expected %d EINVAL", + argv[0], r, EINVAL); + exit (EXIT_FAILURE); + } + + nbd_close (nbd); + exit (EXIT_SUCCESS); +} diff --git a/.gitignore b/.gitignore index 6fdb362..1e49413 100644 --- a/.gitignore +++ b/.gitignore @@ -173,6 +173,7 @@ Makefile.in /tests/newstyle-limited /tests/oldstyle /tests/opt-abort +/tests/opt-info /tests/pki/ /tests/read-only-flag /tests/read-write-flag -- 2.28.0