Allow disabling the NBD_OPT_GO protocol option

For backcompat with nbd-server before commit 2ab3a2d

Fixes: gh-66

Author: yoe

man/nbd-client.8.in.sgml

@@ -311,6 +311,41 @@ manpage.1: manpage.sgml
 	  </para>
 	</listitem>
       </varlistentry>
+      <varlistentry>
+        <term><option>-no-optgo</option></term>
+	<term><option>-g</option></term>
+	<listitem>
+	  <para>Disable the use of the NBD_OPT_GO protocol message, and
+	  force the use of NBD_OPT_EXPORT_NAME instead.</para>
+	  <para>
+	    The NBD protocol has two phases: the negotiation phase, and
+	    the transmission phase. To move from negotation to
+	    transmission, older clients sent the NBD_OPT_EXPORT_NAME
+	    message, for which the server could not produce an error
+	    message in case the export name did not exist (or the client
+	    had insufficient permissions to access it). Due to those
+	    limitations, a replacement message NBD_OPT_GO was created
+	    instead, which allows the server to reply with an error in
+	    case of any problems.</para>
+	  <para>
+	    The protocol allows for a server to discard a message which
+	    it does not understand; however, unfortunately some
+	    implementations (including older versions of nbd-server) did
+	    not handle that situation correctly and would get out of
+	    sync with the client when it sent a message which the server
+	    did not understand.</para>
+	  <para>
+	    When sending NBD_OPT_GO, nbd-client will try to do the right
+	    thing and fall back to NBD_OPT_EXPORT_NAME. However, when
+	    the server has the above-described bug, then this does not
+	    work. In such a situation, the client will issue a
+	    diagnostic suggesting the use of this option.
+	  </para>
+	  <para>
+	    Note that there is a corresponding option for nbdtab, too.
+	  </para>
+	</listitem>
+      </varlistentry>
       <varlistentry>
         <term><option>-name</option></term>
 	<term><option>-N</option></term>

nbd-client.c

@@ -461,7 +461,20 @@ void parse_sizes(char *buf, uint64_t *size, uint16_t *flags) {
 	printf("\n");
 }
 
-void negotiate(int *sockp, u64 *rsize64, uint16_t *flags, char* name, uint32_t needed_flags, uint32_t client_flags, uint32_t do_opts, char *certfile, char *keyfile, char *cacertfile, char *tlshostname, bool tls) {
+void send_opt_exportname(int sock, u64 *rsize64, uint16_t *flags, bool can_opt_go, char* name, uint16_t global_flags) {
+	send_request(sock, NBD_OPT_EXPORT_NAME, -1, name);
+	char b[sizeof(*flags) + sizeof(*rsize64)];
+	if(readit(sock, b, sizeof(b)) < 0 && can_opt_go) {
+		err("E: server does not support NBD_OPT_GO and dropped connection after sending NBD_OPT_EXPORT_NAME. Try -g.");
+	}
+	parse_sizes(b, rsize64, flags);
+	if(!global_flags & NBD_FLAG_NO_ZEROES) {
+		char buf[125];
+		readit(sock, buf, 124);
+	}
+}
+
+void negotiate(int *sockp, u64 *rsize64, uint16_t *flags, char* name, uint32_t needed_flags, uint32_t client_flags, uint32_t do_opts, char *certfile, char *keyfile, char *cacertfile, char *tlshostname, bool tls, bool can_opt_go) {
 	u64 magic;
 	uint16_t tmp;
 	uint16_t global_flags;
@@ -594,25 +607,24 @@ void negotiate(int *sockp, u64 *rsize64, uint16_t *flags, char* name, uint32_t n
 		exit(EXIT_SUCCESS);
 	}
 
-	send_info_request(sock, NBD_OPT_GO, 0, NULL, name);
-
 	struct reply *rep = NULL;
 
+	if(!can_opt_go) {
+		send_opt_exportname(sock, rsize64, flags, can_opt_go, name, global_flags);
+		return;
+	}
+
+	send_info_request(sock, NBD_OPT_GO, 0, NULL, name);
+
 	do {
 		if(rep != NULL) free(rep);
 		rep = read_reply(sock);
-		if(rep->reply_type & NBD_REP_FLAG_ERROR) {
+		if(rep && (rep->reply_type & NBD_REP_FLAG_ERROR)) {
 			switch(rep->reply_type) {
 				case NBD_REP_ERR_UNSUP:
 					/* server doesn't support NBD_OPT_GO or NBD_OPT_INFO,
 					 * fall back to NBD_OPT_EXPORT_NAME */
-					send_request(sock, NBD_OPT_EXPORT_NAME, -1, name);
-					char b[sizeof(*flags) + sizeof(*rsize64)];
-					readit(sock, b, sizeof(b));
-					parse_sizes(b, rsize64, flags);
-					if(!(global_flags & NBD_FLAG_NO_ZEROES)) {
-						readit(sock, buf, 124);
-					}
+					send_opt_exportname(sock, rsize64, flags, can_opt_go, name, global_flags);
 					free(rep);
 					return;
 				case NBD_REP_ERR_POLICY:
@@ -654,13 +666,13 @@ void negotiate(int *sockp, u64 *rsize64, uint16_t *flags, char* name, uint32_t n
 			case NBD_REP_ACK:
 				break;
 			default:
-				err_nonfatal("Unknown reply to NBD_OPT_GO received");
+				err_nonfatal("Unknown reply to NBD_OPT_GO received, ignoring");
 		}
 	} while(rep->reply_type != NBD_REP_ACK);
 	free(rep);
 }
 
-bool get_from_config(char* cfgname, char** name_ptr, char** dev_ptr, char** hostn_ptr, int* bs, int* timeout, int* persist, int* swap, int* sdp, int* b_unix, char**port, int* num_conns, char **certfile, char **keyfile, char **cacertfile, char **tlshostname) {
+bool get_from_config(char* cfgname, char** name_ptr, char** dev_ptr, char** hostn_ptr, int* bs, int* timeout, int* persist, int* swap, int* sdp, int* b_unix, char**port, int* num_conns, char **certfile, char **keyfile, char **cacertfile, char **tlshostname, bool can_opt_go) {
 	int fd = open(SYSCONFDIR "/nbdtab", O_RDONLY);
 	bool retval = false;
 	if(fd < 0) {
@@ -769,6 +781,10 @@ bool get_from_config(char* cfgname, char** name_ptr, char** dev_ptr, char** host
 			*tlshostname = strndup(loc+9, strcspn(loc+9, ","));
 			goto next;
 		}
+		if(!strncmp(loc, "no_optgo", 8)) {
+			*can_opt_go = false;
+			goto next;
+		}
 		// skip unknown options, with a warning unless they start with a '_'
 		l = strcspn(loc, ",");
 		if(*loc != '_') {
@@ -918,7 +934,7 @@ void disconnect(char* device) {
 #if HAVE_NETLINK
 static const char *short_opts = "-A:b:c:C:d:H:hK:LlnN:pSst:uVx";
 #else
-static const char *short_opts = "-A:b:c:C:d:H:hK:lnN:pSst:uVx";
+static const char *short_opts = "-A:b:c:C:d:gH:hK:lnN:pSst:uVx";
 #endif
 
 int main(int argc, char *argv[]) {
@@ -958,6 +974,7 @@ int main(int argc, char *argv[]) {
 		{ "check", required_argument, NULL, 'c' },
 		{ "connections", required_argument, NULL, 'C'},
 		{ "disconnect", required_argument, NULL, 'd' },
+		{ "no-optgo", no_argument, NULL, 'g' },
 		{ "help", no_argument, NULL, 'h' },
 		{ "list", no_argument, NULL, 'l' },
 		{ "name", required_argument, NULL, 'N' },
@@ -980,6 +997,7 @@ int main(int argc, char *argv[]) {
 		{ 0, 0, 0, 0 }, 
 	};
 	int i;
+	bool can_opt_go = true;
 
 	logging(MY_NAME);
 
@@ -1043,6 +1061,9 @@ int main(int argc, char *argv[]) {
 			need_disconnect = 1;
 			nbddev = strdup(optarg);
 			break;
+		case 'g':
+			can_opt_go = false;
+			break;
 		case 'h':
 			usage(NULL);
 			exit(EXIT_SUCCESS);
@@ -1179,7 +1200,7 @@ int main(int argc, char *argv[]) {
 		if (sock < 0)
 			exit(EXIT_FAILURE);
 
-		negotiate(&sock, &size64, &flags, name, needed_flags, cflags, opts, certfile, keyfile, cacertfile, tlshostname, tls);
+		negotiate(&sock, &size64, &flags, name, needed_flags, cflags, opts, certfile, keyfile, cacertfile, tlshostname, tls, can_opt_go);
 		if (netlink) {
 			sockfds[i] = sock;
 			continue;
@@ -1293,7 +1314,7 @@ int main(int argc, char *argv[]) {
 					nbd = open(nbddev, O_RDWR);
 					if (nbd < 0)
 						err("Cannot open NBD: %m");
-					negotiate(&sock, &new_size, &new_flags, name, needed_flags, cflags, opts, certfile, keyfile, cacertfile, tlshostname, tls);
+					negotiate(&sock, &new_size, &new_flags, name, needed_flags, cflags, opts, certfile, keyfile, cacertfile, tlshostname, tls, can_opt_go);
 					if (size64 != new_size) {
 						err("Size of the device changed. Bye");
 					}