|
From: | Vladimir Sementsov-Ogievskiy |
Subject: | Re: [Qemu-block] [PATCH v2] nbd: Flip qemu-nbd to prefer newstyle; add -O for old behavior |
Date: | Wed, 3 Oct 2018 18:21:46 +0000 |
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 <address@hidden> --- 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=NAME expose export by name\n" -" -D, --description=TEXT with -x, also export a human-readable description\n" +" -x, --export-name=NAME expose export by name (default "")\n" +" -D, --description=TEXT expose a human-readable description of export\n" +" -O, --oldstyle force 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 <address@hidden> 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) { -- Best regards, Vladimir |
[Prev in Thread] | Current Thread | [Next in Thread] |