[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH 11/14] nbd/client: Add nbd_receive_export_list()

From: Eric Blake
Subject: Re: [Qemu-block] [PATCH 11/14] nbd/client: Add nbd_receive_export_list()
Date: Fri, 7 Dec 2018 09:19:58 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.1

On 12/7/18 4:04 AM, Vladimir Sementsov-Ogievskiy wrote:
01.12.2018 1:03, Eric Blake wrote:
We want to be able to detect whether a given qemu NBD server is
exposing the right export(s) and dirty bitmaps, at least for
regression testing.  We could use 'nbd-client -l' from the upstream
NBD project to list exports, but it's annoying to rely on
out-of-tree binaries; furthermore, nbd-client doesn't necessarily
know about all of the qemu NBD extensions.  Thus, we plan on adding
a new mode to qemu-nbd that merely sniffs all possible information
from the server during handshake phase, then disconnects and dumps
the information.

This patch adds the low-level client code for grabbing the list
of exports.  It benefits from the recent refactoring patches, as
well as a minor tweak of changing nbd_opt_go() to nbd_opt_info_go(),
in order to share as much code as possible when it comes to doing
validation of server replies.  The resulting information is stored
in an array of NBDExportInfo which has been expanded to hold more
members, along with a convenience function for freeing the list.

Signed-off-by: Eric Blake <address@hidden>

+/* Query details about a server's exports, then disconnect without
+ * going into transmission phase. Return a count of the exports listed
+ * in @info by the server, or -1 on error. Caller must free @info. */
+int nbd_receive_export_list(QIOChannel *ioc, QCryptoTLSCreds *tlscreds,
+                            const char *hostname, NBDExportInfo **info,
+                            Error **errp)
+    int result;
+    int count = 0;
+    int i;
+    int rc;
+    int ret = -1;
+    NBDExportInfo *array = NULL;
+    uint32_t oldflags;
+    QIOChannel *sioc = NULL;

it's a bit confusing that you use sioc name for the second produced ioc, when
in nbd_client_init it's visa-versa.

So, I assume that you mean Secure ioc, when in nbd_client_init it is Socket ioc.

I can s/sioc/outioc/ if that helps. Basically, the output ioc is the same as the original ioc for plaintext, and a secure wrapper socket for tls.

+    case 0: /* oldstyle, parse length and flags */
+        array = g_new0(NBDExportInfo, 1);
+        array->name = g_strdup("");
+        count = 1;
+        if (nbd_read(ioc, &array->size, sizeof(array->size), errp) < 0) {
+            error_prepend(errp, "Failed to read export length: ");
+            goto out;
+        }
+        array->size = be64_to_cpu(array->size);
+        if (nbd_read(ioc, &oldflags, sizeof(oldflags), errp) < 0) {
+            error_prepend(errp, "Failed to read export flags: ");
+            goto out;
+        }
+        oldflags = be32_to_cpu(oldflags);
+        if (oldflags & ~0xffff) {
+            error_setg(errp, "Unexpected export flags %0x" PRIx32, oldflags);
+            goto out;
+        }
+        array->flags = oldflags;

this is a common part of nbd_receive_export_list and nbd_receive_negotiate,
can we move it to nbd_start_negotiate?

Not quite, but I could probably create yet another helper function.

+ out:
+    qio_channel_shutdown(ioc, QIO_CHANNEL_SHUTDOWN_BOTH, NULL);
+    qio_channel_close(ioc, NULL);

interesting, why we don't close it (only shutdown) in other nbd code..

I presume you mean block/nbd-client.c:nbd_teardown_connection, which indeed oncly calls qio_channel_shutdown() followed by object_unref(). I think that unref'ing a channel implies a close, but if not, then that code is leaking an fd (shutdown ends the connection, but does not close resources). I'll have to debug that, but it's independent of this patch.

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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]