From 0095b003b56894baad8e6789d523cf3c51905c05 Mon Sep 17 00:00:00 2001 From: "Richard W.M. Jones" Date: Tue, 17 May 2016 19:43:16 +0100 Subject: [PATCH 10/11] Implement newstyle export names. This is now required by qemu >= 2.6. See this lengthy qemu-devel thread for details: https://lists.nongnu.org/archive/html/qemu-devel/2016-05/threads.html#02752 --- TODO | 13 ------------- docs/nbdkit.pod | 52 ++++++++++++++++++++++++++++++++------------------- src/connections.c | 49 ++++++++++++++++++++++++++++++++++++++++-------- src/internal.h | 1 + src/main.c | 32 +++++++++++++++++++++++-------- src/protocol.h | 3 +-- tests/test-newstyle.c | 7 +++++-- 7 files changed, 105 insertions(+), 52 deletions(-) diff --git a/TODO b/TODO index 177a07c..30bf72e 100644 --- a/TODO +++ b/TODO @@ -11,19 +11,6 @@ * Performance - measure and improve it. -* Implement export names. With export names it should be possible to - have multiple plugins on the command line (each responding to a - different export of course): - - nbdkit --export /foo plugin.so --export /bar another-plugin.so - - Note it should also be possible to either elect one plugin as the - default that accepts all exportnames, or to divide the export name - "space" up using regexps or wildcards. - - Export names are not actually paths (although that is how they are - often used), but arbitrary UTF-8 text strings. - * Implement true parallel request handling. Currently NBDKIT_THREAD_MODEL_SERIALIZE_REQUESTS and NBDKIT_THREAD_MODEL_PARALLEL are the same, because we handle diff --git a/docs/nbdkit.pod b/docs/nbdkit.pod index 7204a38..728aad3 100644 --- a/docs/nbdkit.pod +++ b/docs/nbdkit.pod @@ -6,7 +6,7 @@ nbdkit - A toolkit for creating NBD servers =head1 SYNOPSIS - nbdkit [--dump-config] [-f] [-g GROUP] [-i IPADDR] + nbdkit [--dump-config] [-e EXPORTNAME] [-f] [-g GROUP] [-i IPADDR] [--newstyle] [--oldstyle] [-P PIDFILE] [-p PORT] [-r] [--run CMD] [-s] [-U SOCKET] [-u USER] [-v] [-V] PLUGIN [key=value [key=value [...]]] @@ -74,6 +74,19 @@ Display brief command line usage information and exit. Dump out the compile-time configuration values and exit. +=item B<-e> EXPORTNAME + +=item B<--export> EXPORTNAME + +=item B<--export-name> EXPORTNAME + +=item B<--exportname> EXPORTNAME + +Set the exportname and use the newstyle protocol (implies I<-n>). + +If not set, exportname C<""> (empty string) is used. Exportnames are +not allowed with the oldstyle protocol. + =item B<-f> =item B<--foreground> @@ -118,9 +131,10 @@ protocol. See L below. =item B<--oldstyle> -Use the oldstyle NBD protocol. This is currently the default, so this -flag does nothing, but it is possible we might change the default -protocol in future. See L below. +Use the oldstyle NBD protocol. This is currently the default (unless +you use I<-n> or I<-e>), so this flag does nothing, but it is possible +we might change the default protocol in future. See L below. =item B<-P> PIDFILE @@ -307,27 +321,29 @@ use depends on the client and cannot be known in advance, nor can it be negotiated from the server side. nbdkit currently defaults to the oldstyle protocol for compatibility -with qemu and libguestfs. This is also the same behaviour as -qemu-nbd. Use the I<-n> or I<--newstyle> flag on the command line to -use the newstyle protocol. Use the I<-o> or I<--oldstyle> flag to -force the oldstyle protocol. +with qemu and libguestfs. This is also the same behaviour as qemu-nbd +E 2.5. Use the I<-n> or I<--newstyle> flag on the command line to +use the newstyle protocol. Use the I<-e> or I<--exportname> flag to +set the exportname for the newstyle protocol. Use the I<-o> or +I<--oldstyle> flag to force the oldstyle protocol. Some common clients and the protocol they require: - Client Protocol + Client Protocol ------------------------------------------------------------ - qemu without exportname oldstyle - qemu with exportname newstyle - nbd-client < 3.10 client can talk either protocol - nbd-client >= 3.10 newstyle + qemu <= 2.5 without exportname oldstyle + qemu <= 2.5 with exportname newstyle + qemu >= 2.6 client can talk either protocol + nbd-client < 3.10 client can talk either protocol + nbd-client >= 3.10 newstyle -If you use qemu without the exportname field against a newstyle -server, it will give the error: +If you use qemu E 2.5 without the exportname field against a +newstyle server, it will give the error: Server requires an export name -If you use qemu with the exportname field against an oldstyle server, -it will give the error: +If you use qemu E 2.5 with the exportname field against an +oldstyle server, it will give the error: Server does not support export names @@ -341,8 +357,6 @@ document says should be the case (which isn't based in reality), then you should always use newstyle when using port 10809, and use oldstyle on all other ports. -nbdkit ignores export names at present (see also the C file). - =head1 SIGNALS C responds to the following signals: diff --git a/src/connections.c b/src/connections.c index 34566b3..0c93f35 100644 --- a/src/connections.c +++ b/src/connections.c @@ -222,13 +222,8 @@ _negotiate_handshake_oldstyle (struct connection *conn) return 0; } -/* Receive newstyle options. - * - * Currently we ignore NBD_OPT_EXPORT_NAME (see TODO), we close the - * connection if sent NBD_OPT_ABORT, we send a canned list of - * options for NBD_OPT_LIST, and we send NBD_REP_ERR_UNSUP for - * everything else. - */ +/* Receive newstyle options. */ + static int send_newstyle_option_reply (struct connection *conn, uint32_t option, uint32_t reply) @@ -250,6 +245,39 @@ send_newstyle_option_reply (struct connection *conn, } static int +send_newstyle_option_reply_exportname (struct connection *conn, + uint32_t option, uint32_t reply, + const char *exportname) +{ + struct fixed_new_option_reply fixed_new_option_reply; + size_t name_len = strlen (exportname); + uint32_t len; + + fixed_new_option_reply.magic = htobe64 (NBD_REP_MAGIC); + fixed_new_option_reply.option = htobe32 (option); + fixed_new_option_reply.reply = htobe32 (reply); + fixed_new_option_reply.replylen = htobe32 (name_len + sizeof (len)); + + if (xwrite (conn->sockout, + &fixed_new_option_reply, sizeof fixed_new_option_reply) == -1) { + nbdkit_error ("write: %m"); + return -1; + } + + len = htobe32 (name_len); + if (xwrite (conn->sockout, &len, sizeof len) == -1) { + nbdkit_error ("write: %m"); + return -1; + } + if (xwrite (conn->sockout, exportname, name_len) == -1) { + nbdkit_error ("write: %m"); + return -1; + } + + return 0; +} + +static int _negotiate_handshake_newstyle_options (struct connection *conn) { struct new_option new_option; @@ -309,7 +337,12 @@ _negotiate_handshake_newstyle_options (struct connection *conn) continue; } - /* Since we don't support export names, there is nothing to list. */ + /* Send back the exportname. */ + debug ("newstyle negotiation: advertising export '%s'", exportname); + if (send_newstyle_option_reply_exportname (conn, option, NBD_REP_SERVER, + exportname) == -1) + return -1; + if (send_newstyle_option_reply (conn, option, NBD_REP_ACK) == -1) return -1; break; diff --git a/src/internal.h b/src/internal.h index 0603779..f58086a 100644 --- a/src/internal.h +++ b/src/internal.h @@ -48,6 +48,7 @@ #endif /* main.c */ +extern const char *exportname; extern const char *ipaddr; extern int newstyle; extern const char *port; diff --git a/src/main.c b/src/main.c index db4361e..9529f19 100644 --- a/src/main.c +++ b/src/main.c @@ -65,6 +65,7 @@ static void fork_into_background (void); static uid_t parseuser (const char *); static gid_t parsegroup (const char *); +const char *exportname; /* -e */ int foreground; /* -f */ const char *ipaddr; /* -i */ int newstyle; /* -n */ @@ -84,10 +85,13 @@ static char *random_fifo = NULL; enum { HELP_OPTION = CHAR_MAX + 1 }; -static const char *short_options = "fg:i:nop:P:rsu:U:vV"; +static const char *short_options = "e:fg:i:nop:P:rsu:U:vV"; static const struct option long_options[] = { { "help", 0, NULL, HELP_OPTION }, { "dump-config",0, NULL, 0 }, + { "export", 1, NULL, 'e' }, + { "export-name",1, NULL, 'e' }, + { "exportname", 1, NULL, 'e' }, { "foreground", 0, NULL, 'f' }, { "no-fork", 0, NULL, 'f' }, { "group", 1, NULL, 'g' }, @@ -115,7 +119,7 @@ static const struct option long_options[] = { static void usage (void) { - printf ("nbdkit [--dump-config] [-f] [-g GROUP] [-i IPADDR]\n" + printf ("nbdkit [--dump-config] [-e EXPORTNAME] [-f] [-g GROUP] [-i IPADDR]\n" " [--newstyle] [--oldstyle] [-P PIDFILE] [-p PORT] [-r]\n" " [--run CMD] [-s] [-U SOCKET] [-u USER] [-v] [-V]\n" " PLUGIN [key=value [key=value [...]]]\n" @@ -173,6 +177,11 @@ main (int argc, char *argv[]) } break; + case 'e': + exportname = optarg; + newstyle = 1; + break; + case 'f': foreground = 1; break; @@ -190,9 +199,6 @@ main (int argc, char *argv[]) break; case 'o': - /* XXX When we add support for exportnames, we will need to - * ensure that the user does not use -o + --export. - */ newstyle = 0; break; @@ -263,12 +269,22 @@ main (int argc, char *argv[]) exit (EXIT_FAILURE); } + /* Oldstyle protocol + exportname not allowed. */ + if (newstyle == 0 && exportname != NULL) { + fprintf (stderr, + "%s: cannot use oldstyle protocol (-o) and exportname (-e)\n", + program_name); + exit (EXIT_FAILURE); + } + + /* If exportname was not set on the command line, use "". */ + if (exportname == NULL) + exportname = ""; + /* Remaining command line arguments define the plugins and plugin * configuration. If --help or --version was specified, we still * partially parse these in order that we can display the per-plugin - * help/version information. In future (when the new protocol and - * export names are permitted) we will allow multiple plugins to be - * given, but at the moment only one plugin is allowed. + * help/version information. */ while (optind < argc) { const char *filename = argv[optind]; diff --git a/src/protocol.h b/src/protocol.h index 71a8098..c885d9e 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -70,8 +70,7 @@ struct fixed_new_option_reply { uint64_t magic; /* NBD_REP_MAGIC, network byte order */ uint32_t option; /* option we are replying to */ uint32_t reply; /* NBD_REP_* */ - uint32_t replylen; /* we always send zero at the moment */ - /* reply data follows, but we currently never send any */ + uint32_t replylen; } __attribute__((packed)); #define NBD_REP_MAGIC UINT64_C(0x3e889045565a9) diff --git a/tests/test-newstyle.c b/tests/test-newstyle.c index b1d8ce7..adcc568 100644 --- a/tests/test-newstyle.c +++ b/tests/test-newstyle.c @@ -44,6 +44,8 @@ #include "test.h" +#define EXPORTNAME "/" + int main (int argc, char *argv[]) { @@ -52,7 +54,8 @@ main (int argc, char *argv[]) char *data; size_t i, size; - if (test_start_nbdkit ("-n", NBDKIT_PLUGIN ("file"), "file=file-data", + if (test_start_nbdkit ("-e", EXPORTNAME, + "-n", NBDKIT_PLUGIN ("file"), "file=file-data", NULL) == -1) exit (EXIT_FAILURE); @@ -63,7 +66,7 @@ main (int argc, char *argv[]) } /* Using any exportname causes qemu to use the newstyle protocol. */ - r = guestfs_add_drive_opts (g, "/" /* exportname */, + r = guestfs_add_drive_opts (g, EXPORTNAME, GUESTFS_ADD_DRIVE_OPTS_FORMAT, "raw", GUESTFS_ADD_DRIVE_OPTS_PROTOCOL, "nbd", GUESTFS_ADD_DRIVE_OPTS_SERVER, server, -- 2.7.4