* [Qemu-devel] [PATCH 0/2] nbd server: drop old-style negotiation @ 2018-10-03 17:02 Vladimir Sementsov-Ogievskiy 2018-10-03 17:02 ` [Qemu-devel] [PATCH 1/2] qemu-nbd: " Vladimir Sementsov-Ogievskiy ` (3 more replies) 0 siblings, 4 replies; 12+ messages in thread From: Vladimir Sementsov-Ogievskiy @ 2018-10-03 17:02 UTC (permalink / raw) To: qemu-devel, qemu-block Cc: pbonzini, eblake, mreitz, kwolf, vsementsov, den, rjones, berrange It's unexpected behavior that without -x option qemu-nbd do old-style negotiation. Let's use "" as a default name instead (as it is already done if tls is used) and therefore, drop old-style negotiation from Qemu NBD server. Vladimir Sementsov-Ogievskiy (2): qemu-nbd: drop old-style negotiation nbd/server: drop old-style negotiation include/block/nbd.h | 3 +-- blockdev-nbd.c | 2 +- nbd/server.c | 53 +++++++++++++-------------------------------- qemu-nbd.c | 25 +++++---------------- 4 files changed, 23 insertions(+), 60 deletions(-) -- 2.18.0 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH 1/2] qemu-nbd: drop old-style negotiation 2018-10-03 17:02 [Qemu-devel] [PATCH 0/2] nbd server: drop old-style negotiation Vladimir Sementsov-Ogievskiy @ 2018-10-03 17:02 ` Vladimir Sementsov-Ogievskiy 2018-10-03 17:31 ` Eric Blake 2018-10-03 17:02 ` [Qemu-devel] [PATCH 2/2] nbd/server: " Vladimir Sementsov-Ogievskiy ` (2 subsequent siblings) 3 siblings, 1 reply; 12+ messages in thread From: Vladimir Sementsov-Ogievskiy @ 2018-10-03 17:02 UTC (permalink / raw) To: qemu-devel, qemu-block Cc: pbonzini, eblake, mreitz, kwolf, vsementsov, den, rjones, berrange Use new-style negotiation always, with default "" (empty) export name if it is not specified with '-x' option. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- qemu-nbd.c | 25 ++++++------------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/qemu-nbd.c b/qemu-nbd.c index 51b9d38c72..57e79e30ea 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -56,7 +56,6 @@ #define MBR_SIZE 512 static NBDExport *exp; -static bool newproto; static int verbose; static char *srcpath; static SocketAddress *saddr; @@ -84,8 +83,8 @@ 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 is empty string)\n" +" -D, --description=TEXT export a human-readable description\n" "\n" "Exposing part of the image:\n" " -o, --offset=OFFSET offset into the image\n" @@ -354,8 +353,7 @@ static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc, nb_fds++; nbd_update_server_watch(); - nbd_client_new(newproto ? NULL : exp, cioc, - tlscreds, NULL, nbd_client_closed); + nbd_client_new(NULL, cioc, tlscreds, NULL, nbd_client_closed); } static void nbd_update_server_watch(void) @@ -549,7 +547,7 @@ int main(int argc, char **argv) Error *local_err = NULL; BlockdevDetectZeroesOptions detect_zeroes = BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF; QDict *options = NULL; - const char *export_name = NULL; + const char *export_name = ""; /* Default export name */ const char *export_description = NULL; const char *tlscredsid = NULL; bool imageOpts = false; @@ -808,11 +806,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,14 +1006,8 @@ int main(int argc, char **argv) error_report_err(local_err); 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); - } + nbd_export_set_name(exp, export_name); + nbd_export_set_description(exp, export_description); if (device) { int ret; -- 2.18.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] qemu-nbd: drop old-style negotiation 2018-10-03 17:02 ` [Qemu-devel] [PATCH 1/2] qemu-nbd: " Vladimir Sementsov-Ogievskiy @ 2018-10-03 17:31 ` Eric Blake 0 siblings, 0 replies; 12+ messages in thread From: Eric Blake @ 2018-10-03 17:31 UTC (permalink / raw) To: Vladimir Sementsov-Ogievskiy, qemu-devel, qemu-block Cc: pbonzini, mreitz, kwolf, den, rjones, berrange On 10/3/18 12:02 PM, Vladimir Sementsov-Ogievskiy wrote: > Use new-style negotiation always, with default "" (empty) export name > if it is not specified with '-x' option. If we like this approach (over mine of keeping oldstyle, but via an explicit -O option), then this commit message should add the following: 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: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > qemu-nbd.c | 25 ++++++------------------- > 1 file changed, 6 insertions(+), 19 deletions(-) > If we like your patch over mine, Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org ^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH 2/2] nbd/server: drop old-style negotiation 2018-10-03 17:02 [Qemu-devel] [PATCH 0/2] nbd server: drop old-style negotiation Vladimir Sementsov-Ogievskiy 2018-10-03 17:02 ` [Qemu-devel] [PATCH 1/2] qemu-nbd: " Vladimir Sementsov-Ogievskiy @ 2018-10-03 17:02 ` Vladimir Sementsov-Ogievskiy 2018-10-03 17:35 ` Eric Blake 2018-10-03 17:32 ` [Qemu-devel] [PATCH 0/2] nbd server: " Eric Blake 2018-10-03 20:44 ` Eric Blake 3 siblings, 1 reply; 12+ messages in thread From: Vladimir Sementsov-Ogievskiy @ 2018-10-03 17:02 UTC (permalink / raw) To: qemu-devel, qemu-block Cc: pbonzini, eblake, mreitz, kwolf, vsementsov, den, rjones, berrange After the previous commit, nbd_client_new first parameter is always NULL. Let's drop it with all corresponding old-style negotiation code path which is unreachable now. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- include/block/nbd.h | 3 +-- blockdev-nbd.c | 2 +- nbd/server.c | 53 +++++++++++++-------------------------------- qemu-nbd.c | 2 +- 4 files changed, 18 insertions(+), 42 deletions(-) diff --git a/include/block/nbd.h b/include/block/nbd.h index 4638c839f5..0129d1a4b4 100644 --- a/include/block/nbd.h +++ b/include/block/nbd.h @@ -308,8 +308,7 @@ void nbd_export_set_name(NBDExport *exp, const char *name); void nbd_export_set_description(NBDExport *exp, const char *description); void nbd_export_close_all(void); -void nbd_client_new(NBDExport *exp, - QIOChannelSocket *sioc, +void nbd_client_new(QIOChannelSocket *sioc, QCryptoTLSCreds *tlscreds, const char *tlsaclname, void (*close_fn)(NBDClient *, bool)); diff --git a/blockdev-nbd.c b/blockdev-nbd.c index 1ef11041a7..8913d8ff73 100644 --- a/blockdev-nbd.c +++ b/blockdev-nbd.c @@ -36,7 +36,7 @@ static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc, gpointer opaque) { qio_channel_set_name(QIO_CHANNEL(cioc), "nbd-server"); - nbd_client_new(NULL, cioc, + nbd_client_new(cioc, nbd_server->tlscreds, NULL, nbd_blockdev_client_closed); } diff --git a/nbd/server.c b/nbd/server.c index 1fba21414b..e87f881525 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -1253,7 +1253,6 @@ static coroutine_fn int nbd_negotiate(NBDClient *client, Error **errp) const uint16_t myflags = (NBD_FLAG_HAS_FLAGS | NBD_FLAG_SEND_TRIM | NBD_FLAG_SEND_FLUSH | NBD_FLAG_SEND_FUA | NBD_FLAG_SEND_WRITE_ZEROES | NBD_FLAG_SEND_CACHE); - bool oldStyle; /* Old style negotiation header, no room for options [ 0 .. 7] passwd ("NBDMAGIC") @@ -1274,33 +1273,19 @@ static coroutine_fn int nbd_negotiate(NBDClient *client, Error **errp) trace_nbd_negotiate_begin(); memcpy(buf, "NBDMAGIC", 8); - oldStyle = client->exp != NULL && !client->tlscreds; - if (oldStyle) { - trace_nbd_negotiate_old_style(client->exp->size, - client->exp->nbdflags | myflags); - stq_be_p(buf + 8, NBD_CLIENT_MAGIC); - stq_be_p(buf + 16, client->exp->size); - stl_be_p(buf + 24, client->exp->nbdflags | myflags); + stq_be_p(buf + 8, NBD_OPTS_MAGIC); + stw_be_p(buf + 16, NBD_FLAG_FIXED_NEWSTYLE | NBD_FLAG_NO_ZEROES); - if (nbd_write(client->ioc, buf, sizeof(buf), errp) < 0) { - error_prepend(errp, "write failed: "); - return -EINVAL; - } - } else { - stq_be_p(buf + 8, NBD_OPTS_MAGIC); - stw_be_p(buf + 16, NBD_FLAG_FIXED_NEWSTYLE | NBD_FLAG_NO_ZEROES); - - if (nbd_write(client->ioc, buf, 18, errp) < 0) { - error_prepend(errp, "write failed: "); - return -EINVAL; - } - ret = nbd_negotiate_options(client, myflags, errp); - if (ret != 0) { - if (ret < 0) { - error_prepend(errp, "option negotiation failed: "); - } - return ret; + if (nbd_write(client->ioc, buf, 18, errp) < 0) { + error_prepend(errp, "write failed: "); + return -EINVAL; + } + ret = nbd_negotiate_options(client, myflags, errp); + if (ret != 0) { + if (ret < 0) { + error_prepend(errp, "option negotiation failed: "); } + return ret; } assert(!client->optlen); @@ -2396,13 +2381,8 @@ static void nbd_client_receive_next_request(NBDClient *client) static coroutine_fn void nbd_co_client_start(void *opaque) { NBDClient *client = opaque; - NBDExport *exp = client->exp; Error *local_err = NULL; - if (exp) { - nbd_export_get(exp); - QTAILQ_INSERT_TAIL(&exp->clients, client, next); - } qemu_co_mutex_init(&client->send_lock); if (nbd_negotiate(client, &local_err)) { @@ -2417,13 +2397,11 @@ static coroutine_fn void nbd_co_client_start(void *opaque) } /* - * Create a new client listener on the given export @exp, using the - * given channel @sioc. Begin servicing it in a coroutine. When the - * connection closes, call @close_fn with an indication of whether the - * client completed negotiation. + * Create a new client listener using the given channel @sioc. + * Begin servicing it in a coroutine. When the connection closes, call + * @close_fn with an indication of whether the client completed negotiation. */ -void nbd_client_new(NBDExport *exp, - QIOChannelSocket *sioc, +void nbd_client_new(QIOChannelSocket *sioc, QCryptoTLSCreds *tlscreds, const char *tlsaclname, void (*close_fn)(NBDClient *, bool)) @@ -2433,7 +2411,6 @@ void nbd_client_new(NBDExport *exp, client = g_new0(NBDClient, 1); client->refcount = 1; - client->exp = exp; client->tlscreds = tlscreds; if (tlscreds) { object_ref(OBJECT(client->tlscreds)); diff --git a/qemu-nbd.c b/qemu-nbd.c index 57e79e30ea..aad0b88723 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -353,7 +353,7 @@ static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc, nb_fds++; nbd_update_server_watch(); - nbd_client_new(NULL, cioc, tlscreds, NULL, nbd_client_closed); + nbd_client_new(cioc, tlscreds, NULL, nbd_client_closed); } static void nbd_update_server_watch(void) -- 2.18.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] nbd/server: drop old-style negotiation 2018-10-03 17:02 ` [Qemu-devel] [PATCH 2/2] nbd/server: " Vladimir Sementsov-Ogievskiy @ 2018-10-03 17:35 ` Eric Blake 0 siblings, 0 replies; 12+ messages in thread From: Eric Blake @ 2018-10-03 17:35 UTC (permalink / raw) To: Vladimir Sementsov-Ogievskiy, qemu-devel, qemu-block Cc: pbonzini, mreitz, kwolf, den, rjones, berrange On 10/3/18 12:02 PM, Vladimir Sementsov-Ogievskiy wrote: > After the previous commit, nbd_client_new first parameter is always > NULL. Let's drop it with all corresponding old-style negotiation code > path which is unreachable now. Being able to force oldstyle negotiation for interoperability testing may still be useful. But as fewer and fewer interesting clients exist that want oldstyle (after all, extensions are only usable with newstyle), I'm finding it hard to justify that we need qemu to be the oldstyle server for such interoperability testing (and I can always keep an older qemu binary around). > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > include/block/nbd.h | 3 +-- > blockdev-nbd.c | 2 +- > nbd/server.c | 53 +++++++++++++-------------------------------- > qemu-nbd.c | 2 +- > 4 files changed, 18 insertions(+), 42 deletions(-) > > +++ b/blockdev-nbd.c > @@ -36,7 +36,7 @@ static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc, > gpointer opaque) > { > qio_channel_set_name(QIO_CHANNEL(cioc), "nbd-server"); > - nbd_client_new(NULL, cioc, > + nbd_client_new(cioc, > nbd_server->tlscreds, NULL, Could rewrap into fewer lines, but that's cosmetic. Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] nbd server: drop old-style negotiation 2018-10-03 17:02 [Qemu-devel] [PATCH 0/2] nbd server: drop old-style negotiation Vladimir Sementsov-Ogievskiy 2018-10-03 17:02 ` [Qemu-devel] [PATCH 1/2] qemu-nbd: " Vladimir Sementsov-Ogievskiy 2018-10-03 17:02 ` [Qemu-devel] [PATCH 2/2] nbd/server: " Vladimir Sementsov-Ogievskiy @ 2018-10-03 17:32 ` Eric Blake 2018-10-03 17:59 ` Vladimir Sementsov-Ogievskiy 2018-10-03 20:44 ` Eric Blake 3 siblings, 1 reply; 12+ messages in thread From: Eric Blake @ 2018-10-03 17:32 UTC (permalink / raw) To: Vladimir Sementsov-Ogievskiy, qemu-devel, qemu-block Cc: pbonzini, mreitz, kwolf, den, rjones, berrange On 10/3/18 12:02 PM, Vladimir Sementsov-Ogievskiy wrote: > It's unexpected behavior that without -x option qemu-nbd do old-style > negotiation. Let's use "" as a default name instead (as it is already > done if tls is used) and therefore, drop old-style negotiation from > Qemu NBD server. Oddly enough, I wrote a similar patch in parallel, and am only now just seeing your mail. Yours is a bit stronger than mine (I added 'qemu-nbd -O' to allow explicit fallback to oldstyle, while you ripped it out altogether). The client can negotiate either style, so we don't need an option on the client side; rather, this is all about what the server should do by default. https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg00568.html Does anyone have a preference between the two? Here's the last time it was discussed: https://lists.gnu.org/archive/html/qemu-devel/2018-08/msg03252.html > > Vladimir Sementsov-Ogievskiy (2): > qemu-nbd: drop old-style negotiation > nbd/server: drop old-style negotiation > > include/block/nbd.h | 3 +-- > blockdev-nbd.c | 2 +- > nbd/server.c | 53 +++++++++++++-------------------------------- > qemu-nbd.c | 25 +++++---------------- > 4 files changed, 23 insertions(+), 60 deletions(-) > -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] nbd server: drop old-style negotiation 2018-10-03 17:32 ` [Qemu-devel] [PATCH 0/2] nbd server: " Eric Blake @ 2018-10-03 17:59 ` Vladimir Sementsov-Ogievskiy 2018-10-03 18:08 ` Eric Blake 0 siblings, 1 reply; 12+ messages in thread From: Vladimir Sementsov-Ogievskiy @ 2018-10-03 17:59 UTC (permalink / raw) To: Eric Blake, qemu-devel@nongnu.org, qemu-block@nongnu.org Cc: pbonzini@redhat.com, mreitz@redhat.com, kwolf@redhat.com, Denis Lunev, rjones@redhat.com, berrange@redhat.com 03.10.2018 20:32, Eric Blake wrote: On 10/3/18 12:02 PM, Vladimir Sementsov-Ogievskiy wrote: It's unexpected behavior that without -x option qemu-nbd do old-style negotiation. Let's use "" as a default name instead (as it is already done if tls is used) and therefore, drop old-style negotiation from Qemu NBD server. Oddly enough, I wrote a similar patch in parallel, and am only now just seeing your mail. Yours is a bit stronger than mine (I added 'qemu-nbd -O' to allow explicit fallback to oldstyle, while you ripped it out altogether). The client can negotiate either style, so we don't need an option on the client side; rather, this is all about what the server should do by default. https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg00568.html Does anyone have a preference between the two? Here's the last time it was discussed: https://lists.gnu.org/archive/html/qemu-devel/2018-08/msg03252.html Hm, I think everyone who like new-style should answer that he don't care (except stricter way is a bit better: don't have options and code which we don't need). But if there is someone who has client which support only old-style negotiation his unhappiness (in case of strict way) will outweigh all our "bits":) I'm from the first group). But let's chose your patch Vladimir Sementsov-Ogievskiy (2): qemu-nbd: drop old-style negotiation nbd/server: drop old-style negotiation include/block/nbd.h | 3 +-- blockdev-nbd.c | 2 +- nbd/server.c | 53 +++++++++++++-------------------------------- qemu-nbd.c | 25 +++++---------------- 4 files changed, 23 insertions(+), 60 deletions(-) -- Best regards, Vladimir ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] nbd server: drop old-style negotiation 2018-10-03 17:59 ` Vladimir Sementsov-Ogievskiy @ 2018-10-03 18:08 ` Eric Blake 2018-10-03 20:30 ` Richard W.M. Jones 2018-10-04 12:10 ` Vladimir Sementsov-Ogievskiy 0 siblings, 2 replies; 12+ messages in thread From: Eric Blake @ 2018-10-03 18:08 UTC (permalink / raw) To: Vladimir Sementsov-Ogievskiy, qemu-devel@nongnu.org, qemu-block@nongnu.org Cc: pbonzini@redhat.com, mreitz@redhat.com, kwolf@redhat.com, Denis Lunev, rjones@redhat.com, berrange@redhat.com On 10/3/18 12:59 PM, Vladimir Sementsov-Ogievskiy wrote: > 03.10.2018 20:32, Eric Blake wrote: > On 10/3/18 12:02 PM, Vladimir Sementsov-Ogievskiy wrote: > It's unexpected behavior that without -x option qemu-nbd do old-style > negotiation. Let's use "" as a default name instead (as it is already > done if tls is used) and therefore, drop old-style negotiation from > Qemu NBD server. Hmm, your email quoting style changed from prior emails that used to prepend '>' when quoting, making it harder to tell where the text you are quoting ends,... > > Oddly enough, I wrote a similar patch in parallel, and am only now just seeing your mail. Yours is a bit stronger than mine (I added 'qemu-nbd -O' to allow explicit fallback to oldstyle, while you ripped it out altogether). The client can negotiate either style, so we don't need an option on the client side; rather, this is all about what the server should do by default. > > https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg00568.html > > Does anyone have a preference between the two? Here's the last time it was discussed: > > https://lists.gnu.org/archive/html/qemu-devel/2018-08/msg03252.html > and your reply begins. > Hm, I think everyone who like new-style should answer that he don't care (except stricter way is a bit better: don't have options and code which we don't need). > But if there is someone who has client which support only old-style negotiation his unhappiness (in case of strict way) will outweigh all our "bits":) > > I'm from the first group). But let's chose your patch My argument in favor of your patch over mine: nbdkit is a GREAT testbed for forcing all sorts of integration testing scenarios, including oldstyle servers. Also, it includes a plugin for translating between new and oldstyle at will. That is, if we ever legitimately encounter a client that can only talk oldstyle, but qemu only talks newstyle, we just tell the user to connect: old client => nbdkit -o nbd => newstyle qemu and then qemu doesn't have to worry about oldstyle because nbdkit does instead. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] nbd server: drop old-style negotiation 2018-10-03 18:08 ` Eric Blake @ 2018-10-03 20:30 ` Richard W.M. Jones 2018-10-04 12:10 ` Vladimir Sementsov-Ogievskiy 1 sibling, 0 replies; 12+ messages in thread From: Richard W.M. Jones @ 2018-10-03 20:30 UTC (permalink / raw) To: Eric Blake Cc: Vladimir Sementsov-Ogievskiy, qemu-devel@nongnu.org, qemu-block@nongnu.org, pbonzini@redhat.com, mreitz@redhat.com, kwolf@redhat.com, Denis Lunev, berrange@redhat.com FWIW I don't have anything to add - agree with what's been said already. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] nbd server: drop old-style negotiation 2018-10-03 18:08 ` Eric Blake 2018-10-03 20:30 ` Richard W.M. Jones @ 2018-10-04 12:10 ` Vladimir Sementsov-Ogievskiy 2018-10-04 12:22 ` Richard W.M. Jones 1 sibling, 1 reply; 12+ messages in thread From: Vladimir Sementsov-Ogievskiy @ 2018-10-04 12:10 UTC (permalink / raw) To: Eric Blake, qemu-devel@nongnu.org, qemu-block@nongnu.org Cc: pbonzini@redhat.com, mreitz@redhat.com, kwolf@redhat.com, Denis Lunev, rjones@redhat.com, berrange@redhat.com 03.10.2018 21:08, Eric Blake wrote: > On 10/3/18 12:59 PM, Vladimir Sementsov-Ogievskiy wrote: >> 03.10.2018 20:32, Eric Blake wrote: >> On 10/3/18 12:02 PM, Vladimir Sementsov-Ogievskiy wrote: >> It's unexpected behavior that without -x option qemu-nbd do old-style >> negotiation. Let's use "" as a default name instead (as it is already >> done if tls is used) and therefore, drop old-style negotiation from >> Qemu NBD server. > > Hmm, your email quoting style changed from prior emails that used to > prepend '>' when quoting, making it harder to tell where the text you > are quoting ends,... I didn't change anything... > >> >> Oddly enough, I wrote a similar patch in parallel, and am only now >> just seeing your mail. Yours is a bit stronger than mine (I added >> 'qemu-nbd -O' to allow explicit fallback to oldstyle, while you >> ripped it out altogether). The client can negotiate either style, so >> we don't need an option on the client side; rather, this is all about >> what the server should do by default. >> >> https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg00568.html >> >> Does anyone have a preference between the two? Here's the last time >> it was discussed: >> >> https://lists.gnu.org/archive/html/qemu-devel/2018-08/msg03252.html >> > > and your reply begins. > >> Hm, I think everyone who like new-style should answer that he don't >> care (except stricter way is a bit better: don't have options and >> code which we don't need). >> But if there is someone who has client which support only old-style >> negotiation his unhappiness (in case of strict way) will outweigh all >> our "bits":) >> >> I'm from the first group). But let's chose your patch > > My argument in favor of your patch over mine: nbdkit is a GREAT > testbed for forcing all sorts of integration testing scenarios, > including oldstyle servers. Also, it includes a plugin for > translating between new and oldstyle at will. That is, if we ever > legitimately encounter a client that can only talk oldstyle, but qemu > only talks newstyle, we just tell the user to connect: > > old client => nbdkit -o nbd => newstyle qemu > > and then qemu doesn't have to worry about oldstyle because nbdkit does > instead. > this is a bit more difficult and more overhead than option in Qemu.. But I don't sure we should care about -- Best regards, Vladimir ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] nbd server: drop old-style negotiation 2018-10-04 12:10 ` Vladimir Sementsov-Ogievskiy @ 2018-10-04 12:22 ` Richard W.M. Jones 0 siblings, 0 replies; 12+ messages in thread From: Richard W.M. Jones @ 2018-10-04 12:22 UTC (permalink / raw) To: Vladimir Sementsov-Ogievskiy Cc: Eric Blake, qemu-devel@nongnu.org, qemu-block@nongnu.org, pbonzini@redhat.com, mreitz@redhat.com, kwolf@redhat.com, Denis Lunev, berrange@redhat.com On Thu, Oct 04, 2018 at 12:10:17PM +0000, Vladimir Sementsov-Ogievskiy wrote: > this is a bit more difficult and more overhead than option in Qemu.. But > I don't sure we should care about I think Eric's point is that dropping oldstyle in qemu-nbd isn't really a problem because nbdkit has no plans to drop oldstyle. So there will remain a path for very old clients to keep working (whether using nbdkit natively, or using nbdkit to proxy to qemu-nbd). Eric's example used the more complex proxy case. By contrast the nbdkit native case is very simple: nbdkit -o file disk.img (where the ‘-o’ flag switches to oldstyle protocol). Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/ ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] nbd server: drop old-style negotiation 2018-10-03 17:02 [Qemu-devel] [PATCH 0/2] nbd server: drop old-style negotiation Vladimir Sementsov-Ogievskiy ` (2 preceding siblings ...) 2018-10-03 17:32 ` [Qemu-devel] [PATCH 0/2] nbd server: " Eric Blake @ 2018-10-03 20:44 ` Eric Blake 3 siblings, 0 replies; 12+ messages in thread From: Eric Blake @ 2018-10-03 20:44 UTC (permalink / raw) To: Vladimir Sementsov-Ogievskiy, qemu-devel, qemu-block Cc: pbonzini, mreitz, kwolf, den, rjones, berrange On 10/3/18 12:02 PM, Vladimir Sementsov-Ogievskiy wrote: > It's unexpected behavior that without -x option qemu-nbd do old-style > negotiation. Let's use "" as a default name instead (as it is already > done if tls is used) and therefore, drop old-style negotiation from > Qemu NBD server. > > Vladimir Sementsov-Ogievskiy (2): > qemu-nbd: drop old-style negotiation > nbd/server: drop old-style negotiation > > include/block/nbd.h | 3 +-- > blockdev-nbd.c | 2 +- > nbd/server.c | 53 +++++++++++++-------------------------------- > qemu-nbd.c | 25 +++++---------------- > 4 files changed, 23 insertions(+), 60 deletions(-) > Thanks; applying this series to my NBD queue. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2018-10-04 12:31 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-10-03 17:02 [Qemu-devel] [PATCH 0/2] nbd server: drop old-style negotiation Vladimir Sementsov-Ogievskiy 2018-10-03 17:02 ` [Qemu-devel] [PATCH 1/2] qemu-nbd: " Vladimir Sementsov-Ogievskiy 2018-10-03 17:31 ` Eric Blake 2018-10-03 17:02 ` [Qemu-devel] [PATCH 2/2] nbd/server: " Vladimir Sementsov-Ogievskiy 2018-10-03 17:35 ` Eric Blake 2018-10-03 17:32 ` [Qemu-devel] [PATCH 0/2] nbd server: " Eric Blake 2018-10-03 17:59 ` Vladimir Sementsov-Ogievskiy 2018-10-03 18:08 ` Eric Blake 2018-10-03 20:30 ` Richard W.M. Jones 2018-10-04 12:10 ` Vladimir Sementsov-Ogievskiy 2018-10-04 12:22 ` Richard W.M. Jones 2018-10-03 20:44 ` Eric Blake
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).