Re: [Qemu-devel] [PATCH 2/2] nbd/server: drop old-style negotiation

2018-10-03 Thread Eric Blake

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 
---
  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 

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



[Qemu-devel] [PATCH 2/2] nbd/server: drop old-style negotiation

2018-10-03 Thread Vladimir Sementsov-Ogievskiy
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 
---
 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 *tlsacl