[Qemu-devel] [PATCH v3 09/13] nbd: pick first exported volume if no export name is requested

["Daniel P. Berrange" <berrange@redhat.com>]

The NBD client is currently only capable of using the new style
protocol negotiation if an explicit export name is provided.
This is a problem, because TLS support will require use of the
new style protocol in all cases, and we wish to keep the export
name as an optional request for backwards compatibility.

The trivial way to deal with this is to use the NBD protocol
option for listing exports and then pick the first returned
export as the one to use. This makes the client "do the right
thing" in the normal case where the qemu-nbd server only
exports a single volume.

Furthermore, in order to get proper error reporting of unknown
options with fixed new style protocol, we must not send the
NBD_OPT_EXPORT_NAME as the first thing. Thus, even if an export
name is provided we still send NBD_OPT_LIST to enumerate server
exports. This also gives us clearer error reporting in the case
that the requested export name does not exist. If NBD_OPT_LIST
is not supported, we still fallback to using the specified
export name, so as not to break compatibility with old QEMU
NBD server impls that predated NBD_OPT_LIST

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 nbd/client.c | 201 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 nbd/server.c |   2 +
 2 files changed, 198 insertions(+), 5 deletions(-)

diff --git a/nbd/client.c b/nbd/client.c
index 22d69ff..3c245a5 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -70,12 +70,187 @@ static QTAILQ_HEAD(, NBDExport) exports = QTAILQ_HEAD_INITIALIZER(exports);
 
 */
 
+
+static int nbd_handle_reply_err(uint32_t opt, uint32_t type, Error **errp)
+{
+    if (!(type & (1 << 31))) {
+        return 0;
+    }
+
+    switch (type) {
+    case NBD_REP_ERR_UNSUP:
+        error_setg(errp, "Unsupported option type %x", opt);
+        break;
+
+    case NBD_REP_ERR_INVALID:
+        error_setg(errp, "Invalid data length for option %x", opt);
+        break;
+
+    default:
+        error_setg(errp, "Unknown error code when asking for option %x", opt);
+        break;
+    }
+
+    return -1;
+}
+
+static int nbd_receive_list(QIOChannel *ioc, char **name, Error **errp)
+{
+    uint64_t magic;
+    uint32_t opt;
+    uint32_t type;
+    uint32_t len;
+    uint32_t namelen;
+
+    *name = NULL;
+    if (read_sync(ioc, &magic, sizeof(magic)) != sizeof(magic)) {
+        error_setg(errp, "failed to read list option magic");
+        return -1;
+    }
+    magic = be64_to_cpu(magic);
+    if (magic != NBD_REP_MAGIC) {
+        error_setg(errp, "Unexpected option list magic");
+        return -1;
+    }
+    if (read_sync(ioc, &opt, sizeof(opt)) != sizeof(opt)) {
+        error_setg(errp, "failed to read list option");
+        return -1;
+    }
+    opt = be32_to_cpu(opt);
+    if (opt != NBD_OPT_LIST) {
+        error_setg(errp, "Unexpected option type %x expected %x",
+                   opt, NBD_OPT_LIST);
+        return -1;
+    }
+
+    if (read_sync(ioc, &type, sizeof(type)) != sizeof(type)) {
+        error_setg(errp, "failed to read list option type");
+        return -1;
+    }
+    type = be32_to_cpu(type);
+    if (type == NBD_REP_ERR_UNSUP) {
+        return 0;
+    }
+    if (nbd_handle_reply_err(opt, type, errp) < 0) {
+        return -1;
+    }
+
+    if (read_sync(ioc, &len, sizeof(len)) != sizeof(len)) {
+        error_setg(errp, "failed to read option length");
+        return -1;
+    }
+    len = be32_to_cpu(len);
+
+    if (type == NBD_REP_ACK) {
+        if (len != 0) {
+            error_setg(errp, "length too long for option end");
+            return -1;
+        }
+    } else if (type == NBD_REP_SERVER) {
+        if (read_sync(ioc, &namelen, sizeof(namelen)) != sizeof(namelen)) {
+            error_setg(errp, "failed to read option name length");
+            return -1;
+        }
+        namelen = be32_to_cpu(namelen);
+        if (len != (namelen + sizeof(namelen))) {
+            error_setg(errp, "incorrect option mame length");
+            return -1;
+        }
+        if (namelen > 255) {
+            error_setg(errp, "export name length too long %d", namelen);
+            return -1;
+        }
+
+        *name = g_new0(char, namelen + 1);
+        if (read_sync(ioc, *name, namelen) != namelen) {
+            error_setg(errp, "failed to read export name");
+            g_free(*name);
+            *name = NULL;
+            return -1;
+        }
+        (*name)[namelen] = '\0';
+    } else {
+        error_setg(errp, "Unexpected reply type %x expected %x",
+                   type, NBD_REP_SERVER);
+        return -1;
+    }
+    return 1;
+}
+
+
+static char *nbd_receive_pick_export(QIOChannel *ioc,
+                                     const char *wantname,
+                                     Error **errp)
+{
+    uint64_t magic = cpu_to_be64(NBD_OPTS_MAGIC);
+    uint32_t opt = cpu_to_be32(NBD_OPT_LIST);
+    uint32_t length = 0;
+    char *chosenname = NULL;
+
+    TRACE("Picking export name");
+    if (write_sync(ioc, &magic, sizeof(magic)) != sizeof(magic)) {
+        error_setg(errp, "Failed to send list option magic");
+        goto fail;
+    }
+
+    if (write_sync(ioc, &opt, sizeof(opt)) != sizeof(opt)) {
+        error_setg(errp, "Failed to send list option number");
+        goto fail;
+    }
+
+    if (write_sync(ioc, &length, sizeof(length)) != sizeof(length)) {
+        error_setg(errp, "Failed to send list option length");
+        goto fail;
+    }
+
+    TRACE("Reading available export names");
+    while (1) {
+        char *name = NULL;
+        int ret = nbd_receive_list(ioc, &name, errp);
+
+        if (ret < 0) {
+            g_free(name);
+            name = NULL;
+            goto fail;
+        }
+        if (ret == 0) { /* Server doesn't support export listing */
+            if (wantname == NULL) {
+                error_setg(errp, "Export name is required");
+                goto fail;
+            }
+            chosenname = g_strdup(wantname);
+            break;
+        }
+        if (name == NULL) {
+            TRACE("End of export name list");
+            break;
+        }
+        if (!chosenname && (!wantname || g_str_equal(name, wantname))) {
+            chosenname = name;
+            TRACE("Picked export name %s", name);
+        } else {
+            TRACE("Ignored export name %s", name);
+            g_free(name);
+        }
+    }
+
+    if (wantname && !chosenname) {
+        error_setg(errp, "No export with name %s available", wantname);
+        goto fail;
+    }
+
+ fail:
+    TRACE("Chosen export name %s", chosenname ? chosenname : "<null>");
+    return chosenname;
+}
+
 int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint32_t *flags,
                           off_t *size, Error **errp)
 {
     char buf[256];
     uint64_t magic, s;
     int rc;
+    char *autoname = NULL;
 
     TRACE("Receiving negotiation.");
 
@@ -120,27 +295,40 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint32_t *flags,
         uint32_t namesize;
         uint16_t globalflags;
         uint16_t exportflags;
+        bool fixedNewStyle = false;
 
         if (read_sync(ioc, &globalflags, sizeof(globalflags)) !=
             sizeof(globalflags)) {
             error_setg(errp, "Failed to read server flags");
             goto fail;
         }
-        *flags = be16_to_cpu(globalflags) << 16;
+        globalflags = be16_to_cpu(globalflags);
+        *flags = globalflags << 16;
+        TRACE("Global flags are %x", globalflags);
         if (globalflags & NBD_FLAG_FIXED_NEWSTYLE) {
+            fixedNewStyle = true;
             TRACE("Server supports fixed new style");
             clientflags |= NBD_FLAG_C_FIXED_NEWSTYLE;
         }
         /* client requested flags */
+        clientflags = cpu_to_be32(clientflags);
         if (write_sync(ioc, &clientflags, sizeof(clientflags)) !=
             sizeof(clientflags)) {
             error_setg(errp, "Failed to send clientflags field");
             goto fail;
         }
         /* write the export name */
-        if (!name) {
-            error_setg(errp, "Server requires an export name");
-            goto fail;
+        if (fixedNewStyle) {
+            autoname = nbd_receive_pick_export(ioc, name, errp);
+            if (!autoname) {
+                goto fail;
+            }
+            name = autoname;
+        } else {
+            if (!name) {
+                error_setg(errp, "Server requires an export name");
+                goto fail;
+            }
         }
         magic = cpu_to_be64(magic);
         if (write_sync(ioc, &magic, sizeof(magic)) != sizeof(magic)) {
@@ -175,7 +363,9 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint32_t *flags,
             error_setg(errp, "Failed to read export flags");
             goto fail;
         }
-        *flags |= be16_to_cpu(exportflags);
+        exportflags = be16_to_cpu(exportflags);
+        *flags |= exportflags;
+        TRACE("Export flags are %x", exportflags);
     } else if (magic == NBD_CLIENT_MAGIC) {
         if (name) {
             error_setg(errp, "Server does not support export names");
@@ -206,6 +396,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint32_t *flags,
     rc = 0;
 
 fail:
+    free(autoname);
     return rc;
 }
 
diff --git a/nbd/server.c b/nbd/server.c
index 8343a09..5296808 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -291,6 +291,8 @@ static int nbd_negotiate_handle_export_name(NBDClient *client, uint32_t length)
     }
     name[length] = '\0';
 
+    TRACE("Client requested export '%s'", name);
+
     client->exp = nbd_export_find(name);
     if (!client->exp) {
         LOG("export not found");
-- 
2.5.0