The NBD spec says that if a client requests SET_META_CONTEXT for exportA, but later requests NBD_OPT_GO/EXPORT_NAME for exportB, then the server should reject NBD_CMD_BLOCK_STATUS requests (that is, the context returned for exportA need not apply to exportB). When we originally added base:allocation, our argument was that we always ignore export names, so it was easier to just treat any two export names as being the same export, so no need to reset things. But now that we have nbdkit_export_name(), we are better off obeying the spec.
Note that there are no known clients in the wild that can actually perform this cross-export-name request; this was found by inspection, but I also tested it via strategic gdb breakpoints in qemu-io. I also don't see the point in hacking up libnbd to become such a client. Fixes: 4f303e44 Signed-off-by: Eric Blake <[email protected]> --- server/internal.h | 1 + server/protocol-handshake-newstyle.c | 16 +++++++++++++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/server/internal.h b/server/internal.h index e6a22f1a..97e417f9 100644 --- a/server/internal.h +++ b/server/internal.h @@ -200,6 +200,7 @@ struct connection { size_t nr_handles; char exportname[NBD_MAX_STRING + 1]; + uint32_t exportnamelen; uint32_t cflags; uint16_t eflags; bool using_tls; diff --git a/server/protocol-handshake-newstyle.c b/server/protocol-handshake-newstyle.c index 3b5d144e..2480d7a3 100644 --- a/server/protocol-handshake-newstyle.c +++ b/server/protocol-handshake-newstyle.c @@ -239,8 +239,12 @@ check_export_name (struct connection *conn, uint32_t option, char *buf, assert (exportnamelen < sizeof conn->exportname); if (save) { + if (exportnamelen != conn->exportnamelen || + memcmp (conn->exportname, buf, exportnamelen) != 0) + conn->meta_context_base_allocation = false; memcpy (conn->exportname, buf, exportnamelen); conn->exportname[exportnamelen] = '\0'; + conn->exportnamelen = exportnamelen; } debug ("newstyle negotiation: %s: client requested export '%.*s'", name_of_nbd_opt (option), (int) exportnamelen, buf); @@ -451,7 +455,9 @@ negotiate_handshake_newstyle_options (struct connection *conn) } /* As with NBD_OPT_EXPORT_NAME we print the export name and - * save it in the connection. + * save it in the connection. If an earlier + * NBD_OPT_SET_META_CONTEXT used an export name, it must match + * or else we drop the support for that context. */ if (check_export_name (conn, option, &data[4], exportnamelen, optlen - 6, true) == -1) @@ -574,12 +580,16 @@ negotiate_handshake_newstyle_options (struct connection *conn) continue; } - /* Discard the export name, after validating it. */ + /* Remember the export name: the NBD spec says that if the client + * later uses NBD_OPT_GO on a different export, then the context + * returned here is not usable. + */ memcpy (&exportnamelen, &data[0], 4); exportnamelen = be32toh (exportnamelen); what = "validating export name"; if (check_export_name (conn, option, &data[4], exportnamelen, - optlen - 8, false) == -1) + optlen - 8, + option == NBD_OPT_SET_META_CONTEXT) == -1) goto opt_meta_invalid_option_len; opt_index = 4 + exportnamelen; -- 2.21.0 _______________________________________________ Libguestfs mailing list [email protected] https://www.redhat.com/mailman/listinfo/libguestfs
