subject:"\[Qemu\-devel\] \[PATCH v2\] nbd\: Flip qemu\-nbd to prefer newstyle; add \-O for old behavior"

Re: [Qemu-devel] [PATCH v2] nbd: Flip qemu-nbd to prefer newstyle; add -O for old behavior

2018-10-03 Thread Eric Blake


On 10/3/18 12:55 PM, Eric Blake wrote:

Oldstyle NBD negotiation cannot perform any of the extensions that
we have recently been relying on.  While you can always pass -x ""
to get newstyle negotiation, these days, it is better to just default
to newstyle (with an empty export name) and fall back to oldstyle
only on an explicit request.

qemu as client can manage either style since 2.6.0, commit 69b49502d8

For comparison:

nbdkit 1.3 switched its default to newstyle (Jan 2018):
https://github.com/libguestfs/nbdkit/commit/b2a8aecc
https://github.com/libguestfs/nbdkit/commit/8158e773

nbd 3.10 dropped oldstyle long ago (Mar 2015):
https://github.com/NetworkBlockDevice/nbd/commit/36940193

Signed-off-by: Eric Blake 
---
v2: improve 'qemu-nbd --help', drop redundant variable 'newproto'
---
  qemu-nbd.c | 37 +++--
  1 file changed, 23 insertions(+), 14 deletions(-)


But I still forgot to document it in qemu-nbd.texi.

I'm dropping this patch in favor of Vladimir's stronger series, now that 
Rich has agreed with my observation that nbdkit can bridge the needs of 
any non-qemu client that still only talks oldstyle.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH v2] nbd: Flip qemu-nbd to prefer newstyle; add -O for old behavior

2018-10-03 Thread Vladimir Sementsov-Ogievskiy

03.10.2018 20:55, Eric Blake wrote:

Oldstyle NBD negotiation cannot perform any of the extensions that
we have recently been relying on.  While you can always pass -x ""
to get newstyle negotiation, these days, it is better to just default
to newstyle (with an empty export name) and fall back to oldstyle
only on an explicit request.

qemu as client can manage either style since 2.6.0, commit 69b49502d8

For comparison:

nbdkit 1.3 switched its default to newstyle (Jan 2018):
https://github.com/libguestfs/nbdkit/commit/b2a8aecc
https://github.com/libguestfs/nbdkit/commit/8158e773

nbd 3.10 dropped oldstyle long ago (Mar 2015):
https://github.com/NetworkBlockDevice/nbd/commit/36940193

Signed-off-by: Eric Blake 
---
v2: improve 'qemu-nbd --help', drop redundant variable 'newproto'
---
 qemu-nbd.c | 37 +++--
 1 file changed, 23 insertions(+), 14 deletions(-)

diff --git a/qemu-nbd.c b/qemu-nbd.c
index 51b9d38c727..e35c97e7ca4 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -56,7 +56,7 @@
 #define MBR_SIZE 512

 static NBDExport *exp;
-static bool newproto;
+static bool oldstyle;
 static int verbose;
 static char *srcpath;
 static SocketAddress *saddr;
@@ -84,8 +84,9 @@ static void usage(const char *name)
 "  -e, --shared=NUM  device can be shared by NUM clients (default 
'1')\n"
 "  -t, --persistent  don't exit on the last connection\n"
 "  -v, --verbose display extra debugging information\n"
-"  -x, --export-name=NAMEexpose export by name\n"
-"  -D, --description=TEXTwith -x, also export a human-readable 
description\n"
+"  -x, --export-name=NAMEexpose export by name (default "")\n"
+"  -D, --description=TEXTexpose a human-readable description of export\n"
+"  -O, --oldstyleforce oldstyle (not with -x, -D, or 
--tls-creds)\n"
 "\n"
 "Exposing part of the image:\n"
 "  -o, --offset=OFFSET   offset into the image\n"
@@ -354,7 +355,7 @@ static void nbd_accept(QIONetListener *listener, 
QIOChannelSocket *cioc,

 nb_fds++;
 nbd_update_server_watch();
-nbd_client_new(newproto ? NULL : exp, cioc,
+nbd_client_new(oldstyle ? exp : NULL, cioc,
tlscreds, NULL, nbd_client_closed);
 }

@@ -502,7 +503,7 @@ int main(int argc, char **argv)
 off_t fd_size;
 QemuOpts *sn_opts = NULL;
 const char *sn_id_or_name = NULL;
-const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:tl:x:T:D:";
+const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:tl:x:T:D:O";
 struct option lopt[] = {
 { "help", no_argument, NULL, 'h' },
 { "version", no_argument, NULL, 'V' },
@@ -529,6 +530,7 @@ int main(int argc, char **argv)
 { "object", required_argument, NULL, QEMU_NBD_OPT_OBJECT },
 { "export-name", required_argument, NULL, 'x' },
 { "description", required_argument, NULL, 'D' },
+{ "oldstyle", no_argument, NULL, 'O' },
 { "tls-creds", required_argument, NULL, QEMU_NBD_OPT_TLSCREDS },
 { "image-opts", no_argument, NULL, QEMU_NBD_OPT_IMAGE_OPTS },
 { "trace", required_argument, NULL, 'T' },
@@ -723,6 +725,9 @@ int main(int argc, char **argv)
 case 'D':
 export_description = optarg;
 break;
+case 'O':
+oldstyle = true;
+break;
 case 'v':
 verbose = 1;
 break;
@@ -799,7 +804,16 @@ int main(int argc, char **argv)
 }
 }

+if (!oldstyle && !export_name) {
+/* Set the default NBD protocol export name, to favor new style. */
+export_name = "";
+}

this




+
 if (tlscredsid) {
+if (oldstyle) {
+error_report("TLS is incompatible with oldstyle");
+exit(EXIT_FAILURE);
+}
 if (sockpath) {
 error_report("TLS is only supported with IPv4/IPv6");
 exit(EXIT_FAILURE);
@@ -808,11 +822,6 @@ int main(int argc, char **argv)
 error_report("TLS is not supported with a host device");
 exit(EXIT_FAILURE);
 }
-if (!export_name) {
-/* Set the default NBD protocol export name, since
- * we *must* use new style protocol for TLS */
-export_name = "";
-}
 tlscreds = nbd_get_tls_creds(tlscredsid, &local_err);
 if (local_err) {
 error_report("Failed to get TLS creds %s",
@@ -1013,13 +1022,13 @@ int main(int argc, char **argv)
 error_report_err(local_err);
 exit(EXIT_FAILURE);
 }
+if (oldstyle && (export_name || export_description)) {
+error_report("oldstyle negotiation cannot set export details");
+exit(EXIT_FAILURE);
+}

and this are very simple option checks, so, it is better to place them together 
and after getopt loop, before the other more difficult logic.

but it's a nit-picking, so, with or without:
Reviewed-by: Vladimir Sementsov-Ogievskiy 




 if (expor

Re: [Qemu-devel] [PATCH v2] nbd: Flip qemu-nbd to prefer newstyle; add -O for old behavior

2018-10-03 Thread Vladimir Sementsov-Ogievskiy

03.10.2018 20:55, Eric Blake wrote:
> Oldstyle NBD negotiation cannot perform any of the extensions that
> we have recently been relying on.  While you can always pass -x ""
> to get newstyle negotiation, these days, it is better to just default
> to newstyle (with an empty export name) and fall back to oldstyle
> only on an explicit request.
>
> qemu as client can manage either style since 2.6.0, commit 69b49502d8
>
> For comparison:
>
> nbdkit 1.3 switched its default to newstyle (Jan 2018):
> https://github.com/libguestfs/nbdkit/commit/b2a8aecc
> https://github.com/libguestfs/nbdkit/commit/8158e773
>
> nbd 3.10 dropped oldstyle long ago (Mar 2015):
> https://github.com/NetworkBlockDevice/nbd/commit/36940193
>
> Signed-off-by: Eric Blake 
> ---
> v2: improve 'qemu-nbd --help', drop redundant variable 'newproto'
> ---
>   qemu-nbd.c | 37 +++--
>   1 file changed, 23 insertions(+), 14 deletions(-)
>
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index 51b9d38c727..e35c97e7ca4 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -56,7 +56,7 @@
>   #define MBR_SIZE 512
>
>   static NBDExport *exp;
> -static bool newproto;
> +static bool oldstyle;
>   static int verbose;
>   static char *srcpath;
>   static SocketAddress *saddr;
> @@ -84,8 +84,9 @@ static void usage(const char *name)
>   "  -e, --shared=NUM  device can be shared by NUM clients (default 
> '1')\n"
>   "  -t, --persistent  don't exit on the last connection\n"
>   "  -v, --verbose display extra debugging information\n"
> -"  -x, --export-name=NAMEexpose export by name\n"
> -"  -D, --description=TEXTwith -x, also export a human-readable 
> description\n"
> +"  -x, --export-name=NAMEexpose export by name (default "")\n"
> +"  -D, --description=TEXTexpose a human-readable description of export\n"
> +"  -O, --oldstyleforce oldstyle (not with -x, -D, or 
> --tls-creds)\n"
>   "\n"
>   "Exposing part of the image:\n"
>   "  -o, --offset=OFFSET   offset into the image\n"
> @@ -354,7 +355,7 @@ static void nbd_accept(QIONetListener *listener, 
> QIOChannelSocket *cioc,
>
>   nb_fds++;
>   nbd_update_server_watch();
> -nbd_client_new(newproto ? NULL : exp, cioc,
> +nbd_client_new(oldstyle ? exp : NULL, cioc,
>  tlscreds, NULL, nbd_client_closed);
>   }
>
> @@ -502,7 +503,7 @@ int main(int argc, char **argv)
>   off_t fd_size;
>   QemuOpts *sn_opts = NULL;
>   const char *sn_id_or_name = NULL;
> -const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:tl:x:T:D:";
> +const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:tl:x:T:D:O";
>   struct option lopt[] = {
>   { "help", no_argument, NULL, 'h' },
>   { "version", no_argument, NULL, 'V' },
> @@ -529,6 +530,7 @@ int main(int argc, char **argv)
>   { "object", required_argument, NULL, QEMU_NBD_OPT_OBJECT },
>   { "export-name", required_argument, NULL, 'x' },
>   { "description", required_argument, NULL, 'D' },
> +{ "oldstyle", no_argument, NULL, 'O' },
>   { "tls-creds", required_argument, NULL, QEMU_NBD_OPT_TLSCREDS },
>   { "image-opts", no_argument, NULL, QEMU_NBD_OPT_IMAGE_OPTS },
>   { "trace", required_argument, NULL, 'T' },
> @@ -723,6 +725,9 @@ int main(int argc, char **argv)
>   case 'D':
>   export_description = optarg;
>   break;
> +case 'O':
> +oldstyle = true;
> +break;
>   case 'v':
>   verbose = 1;
>   break;
> @@ -799,7 +804,16 @@ int main(int argc, char **argv)
>   }
>   }
>
> +if (!oldstyle && !export_name) {
> +/* Set the default NBD protocol export name, to favor new style. */
> +export_name = "";
> +}
this


> +
>   if (tlscredsid) {
> +if (oldstyle) {
> +error_report("TLS is incompatible with oldstyle");
> +exit(EXIT_FAILURE);
> +}
>   if (sockpath) {
>   error_report("TLS is only supported with IPv4/IPv6");
>   exit(EXIT_FAILURE);
> @@ -808,11 +822,6 @@ int main(int argc, char **argv)
>   error_report("TLS is not supported with a host device");
>   exit(EXIT_FAILURE);
>   }
> -if (!export_name) {
> -/* Set the default NBD protocol export name, since
> - * we *must* use new style protocol for TLS */
> -export_name = "";
> -}
>   tlscreds = nbd_get_tls_creds(tlscredsid, &local_err);
>   if (local_err) {
>   error_report("Failed to get TLS creds %s",
> @@ -1013,13 +1022,13 @@ int main(int argc, char **argv)
>   error_report_err(local_err);
>   exit(EXIT_FAILURE);
>   }
> +if (oldstyle && (export_name || export_description)) {
> +error_report("oldstyle negotiation cannot set export details");
> +exit(EXIT_FAILURE);
> +}

and this are simple o

[Qemu-devel] [PATCH v2] nbd: Flip qemu-nbd to prefer newstyle; add -O for old behavior

2018-10-03 Thread Eric Blake

Oldstyle NBD negotiation cannot perform any of the extensions that
we have recently been relying on.  While you can always pass -x ""
to get newstyle negotiation, these days, it is better to just default
to newstyle (with an empty export name) and fall back to oldstyle
only on an explicit request.

qemu as client can manage either style since 2.6.0, commit 69b49502d8

For comparison:

nbdkit 1.3 switched its default to newstyle (Jan 2018):
https://github.com/libguestfs/nbdkit/commit/b2a8aecc
https://github.com/libguestfs/nbdkit/commit/8158e773

nbd 3.10 dropped oldstyle long ago (Mar 2015):
https://github.com/NetworkBlockDevice/nbd/commit/36940193

Signed-off-by: Eric Blake 
---
v2: improve 'qemu-nbd --help', drop redundant variable 'newproto'
---
 qemu-nbd.c | 37 +++--
 1 file changed, 23 insertions(+), 14 deletions(-)

diff --git a/qemu-nbd.c b/qemu-nbd.c
index 51b9d38c727..e35c97e7ca4 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -56,7 +56,7 @@
 #define MBR_SIZE 512

 static NBDExport *exp;
-static bool newproto;
+static bool oldstyle;
 static int verbose;
 static char *srcpath;
 static SocketAddress *saddr;
@@ -84,8 +84,9 @@ static void usage(const char *name)
 "  -e, --shared=NUM  device can be shared by NUM clients (default 
'1')\n"
 "  -t, --persistent  don't exit on the last connection\n"
 "  -v, --verbose display extra debugging information\n"
-"  -x, --export-name=NAMEexpose export by name\n"
-"  -D, --description=TEXTwith -x, also export a human-readable 
description\n"
+"  -x, --export-name=NAMEexpose export by name (default "")\n"
+"  -D, --description=TEXTexpose a human-readable description of export\n"
+"  -O, --oldstyleforce oldstyle (not with -x, -D, or 
--tls-creds)\n"
 "\n"
 "Exposing part of the image:\n"
 "  -o, --offset=OFFSET   offset into the image\n"
@@ -354,7 +355,7 @@ static void nbd_accept(QIONetListener *listener, 
QIOChannelSocket *cioc,

 nb_fds++;
 nbd_update_server_watch();
-nbd_client_new(newproto ? NULL : exp, cioc,
+nbd_client_new(oldstyle ? exp : NULL, cioc,
tlscreds, NULL, nbd_client_closed);
 }

@@ -502,7 +503,7 @@ int main(int argc, char **argv)
 off_t fd_size;
 QemuOpts *sn_opts = NULL;
 const char *sn_id_or_name = NULL;
-const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:tl:x:T:D:";
+const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:tl:x:T:D:O";
 struct option lopt[] = {
 { "help", no_argument, NULL, 'h' },
 { "version", no_argument, NULL, 'V' },
@@ -529,6 +530,7 @@ int main(int argc, char **argv)
 { "object", required_argument, NULL, QEMU_NBD_OPT_OBJECT },
 { "export-name", required_argument, NULL, 'x' },
 { "description", required_argument, NULL, 'D' },
+{ "oldstyle", no_argument, NULL, 'O' },
 { "tls-creds", required_argument, NULL, QEMU_NBD_OPT_TLSCREDS },
 { "image-opts", no_argument, NULL, QEMU_NBD_OPT_IMAGE_OPTS },
 { "trace", required_argument, NULL, 'T' },
@@ -723,6 +725,9 @@ int main(int argc, char **argv)
 case 'D':
 export_description = optarg;
 break;
+case 'O':
+oldstyle = true;
+break;
 case 'v':
 verbose = 1;
 break;
@@ -799,7 +804,16 @@ int main(int argc, char **argv)
 }
 }

+if (!oldstyle && !export_name) {
+/* Set the default NBD protocol export name, to favor new style. */
+export_name = "";
+}
+
 if (tlscredsid) {
+if (oldstyle) {
+error_report("TLS is incompatible with oldstyle");
+exit(EXIT_FAILURE);
+}
 if (sockpath) {
 error_report("TLS is only supported with IPv4/IPv6");
 exit(EXIT_FAILURE);
@@ -808,11 +822,6 @@ int main(int argc, char **argv)
 error_report("TLS is not supported with a host device");
 exit(EXIT_FAILURE);
 }
-if (!export_name) {
-/* Set the default NBD protocol export name, since
- * we *must* use new style protocol for TLS */
-export_name = "";
-}
 tlscreds = nbd_get_tls_creds(tlscredsid, &local_err);
 if (local_err) {
 error_report("Failed to get TLS creds %s",
@@ -1013,13 +1022,13 @@ int main(int argc, char **argv)
 error_report_err(local_err);
 exit(EXIT_FAILURE);
 }
+if (oldstyle && (export_name || export_description)) {
+error_report("oldstyle negotiation cannot set export details");
+exit(EXIT_FAILURE);
+}
 if (export_name) {
 nbd_export_set_name(exp, export_name);
 nbd_export_set_description(exp, export_description);
-newproto = true;
-} else if (export_description) {
-error_report("Export description requires an export name");
-exit(EXIT_FAILURE);
 }

 if (device) {
-- 
2.17.1

Re: [Qemu-devel] [PATCH v2] nbd: Flip qemu-nbd to prefer newstyle; add -O for old behavior

Re: [Qemu-devel] [PATCH v2] nbd: Flip qemu-nbd to prefer newstyle; add -O for old behavior

Re: [Qemu-devel] [PATCH v2] nbd: Flip qemu-nbd to prefer newstyle; add -O for old behavior

[Qemu-devel] [PATCH v2] nbd: Flip qemu-nbd to prefer newstyle; add -O for old behavior

4 matches

Site Navigation

Mail list logo

Footer information