On Wed, Jun 05, 2019 at 22:01:07 -0500, Eric Blake wrote:
> Migration always uses a TCP socket for NBD servers, because we don't
> support same-host migration. But upcoming pull-mode incremental backup
> needs to also support a Unix socket, for retrieving the backup from
> the same host. Support this by plumbing virStorageNetHostDef through
> the monitor calls, since that is a nice reusable struct that can track
> both TCP and Unix sockets.
>
> Update qemumonitorjsontest to not only test the response to the
> command, but to actually verify that the command itself uses the two
> correct QMP forms. I'm actually a bit surprised that we are not
> utilizing qemuMonitorTestAddItemVerbatim more frequently.
>
> Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>
> ---
> src/qemu/qemu_monitor.h | 6 ++--
> src/qemu/qemu_monitor_json.h | 3 +-
> src/qemu/qemu_migration.c | 7 ++++-
> src/qemu/qemu_monitor.c | 13 +++++---
> src/qemu/qemu_monitor_json.c | 23 ++++++++++----
> tests/qemumonitorjsontest.c | 58 ++++++++++++++++++++++++++++++++++--
> 6 files changed, 92 insertions(+), 18 deletions(-)
>
> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> index dee594fa66..fa84ff821e 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -1094,9 +1094,9 @@ int qemuMonitorGetObjectProps(qemuMonitorPtr mon,
> char *qemuMonitorGetTargetArch(qemuMonitorPtr mon);
>
> int qemuMonitorNBDServerStart(qemuMonitorPtr mon,
> - const char *host,
> - unsigned int port,
> - const char *tls_alias);
> + const virStorageNetHostDef *server,
> + const char *tls_alias)
> + ATTRIBUTE_NONNULL(2);
> int qemuMonitorNBDServerAdd(qemuMonitorPtr mon,
> const char *deviceID,
> bool writable);
> diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
> index acef1a0a79..e41bdc8c4f 100644
> --- a/src/qemu/qemu_monitor_json.h
> +++ b/src/qemu/qemu_monitor_json.h
> @@ -459,8 +459,7 @@ int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon,
> char *qemuMonitorJSONGetTargetArch(qemuMonitorPtr mon);
>
> int qemuMonitorJSONNBDServerStart(qemuMonitorPtr mon,
> - const char *host,
> - unsigned int port,
> + const virStorageNetHostDef *server,
> const char *tls_alias);
> int qemuMonitorJSONNBDServerAdd(qemuMonitorPtr mon,
> const char *deviceID,
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 32b3040473..267a729c6f 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -380,6 +380,10 @@ qemuMigrationDstStartNBDServer(virQEMUDriverPtr driver,
> unsigned short port = 0;
> char *diskAlias = NULL;
> size_t i;
> + virStorageNetHostDef server = {
> + .name = (char *)listenAddr, /* cast away const */
> + .transport = VIR_STORAGE_NET_HOST_TRANS_TCP,
> + };
>
> if (nbdPort < 0 || nbdPort > USHRT_MAX) {
> virReportError(VIR_ERR_INVALID_ARG, "%s",
> @@ -415,7 +419,8 @@ qemuMigrationDstStartNBDServer(virQEMUDriverPtr driver,
> else if (virPortAllocatorAcquire(driver->migrationPorts, &port) < 0)
> goto exit_monitor;
>
> - if (qemuMonitorNBDServerStart(priv->mon, listenAddr, port, tls_alias) < 0)
> + server.port = port;
> + if (qemuMonitorNBDServerStart(priv->mon, &server, tls_alias) < 0)
> goto exit_monitor;
> }
>
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index 6b731cd91a..187513a986 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -3925,15 +3925,20 @@ qemuMonitorGetSEVCapabilities(qemuMonitorPtr mon,
>
> int
> qemuMonitorNBDServerStart(qemuMonitorPtr mon,
> - const char *host,
> - unsigned int port,
> + const virStorageNetHostDef *server,
> const char *tls_alias)
> {
> - VIR_DEBUG("host=%s port=%u tls_alias=%s", host, port, NULLSTR(tls_alias));
> + /* Peek inside the struct for nicer logging */
> + if (server->transport == VIR_STORAGE_NET_HOST_TRANS_TCP)
> + VIR_DEBUG("server={tcp host=%s port=%u} tls_alias=%s",
> + NULLSTR(server->name), server->port, NULLSTR(tls_alias));
> + else
> + VIR_DEBUG("server={unix socket=%s} tls_alias=%s",
> + NULLSTR(server->socket), NULLSTR(tls_alias));
>
> QEMU_CHECK_MONITOR(mon);
>
> - return qemuMonitorJSONNBDServerStart(mon, host, port, tls_alias);
> + return qemuMonitorJSONNBDServerStart(mon, server, tls_alias);
> }
>
>
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 53a7de8b77..93113d4e8a 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -6684,8 +6684,7 @@ qemuMonitorJSONBuildUnixSocketAddress(const char *path)
>
> int
> qemuMonitorJSONNBDServerStart(qemuMonitorPtr mon,
> - const char *host,
> - unsigned int port,
> + const virStorageNetHostDef *server,
> const char *tls_alias)
> {
> int ret = -1;
> @@ -6694,10 +6693,22 @@ qemuMonitorJSONNBDServerStart(qemuMonitorPtr mon,
> virJSONValuePtr addr = NULL;
> char *port_str = NULL;
>
> - if (virAsprintf(&port_str, "%u", port) < 0)
> - return ret;
> -
> - if (!(addr = qemuMonitorJSONBuildInetSocketAddress(host, port_str)))
> + switch ((virStorageNetHostTransport)server->transport) {
> + case VIR_STORAGE_NET_HOST_TRANS_TCP:
> + if (virAsprintf(&port_str, "%u", server->port) < 0)
> + return ret;
> + addr = qemuMonitorJSONBuildInetSocketAddress(server->name, port_str);
> + break;
> + case VIR_STORAGE_NET_HOST_TRANS_UNIX:
> + addr = qemuMonitorJSONBuildUnixSocketAddress(server->socket);
> + break;
> + case VIR_STORAGE_NET_HOST_TRANS_RDMA:
> + case VIR_STORAGE_NET_HOST_TRANS_LAST:
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("invalid server address"));
> + goto cleanup;
> + }
> + if (!addr)
> goto cleanup;
>
> if (!(cmd = qemuMonitorJSONMakeCommand("nbd-server-start",
> diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
> index 0894e748ae..9d707fcc7c 100644
> --- a/tests/qemumonitorjsontest.c
> +++ b/tests/qemumonitorjsontest.c
> @@ -1348,7 +1348,6 @@ GEN_TEST_FUNC(qemuMonitorJSONBlockCommit, "vdb", "/foo/bar1", "/foo/bar2", "back
> GEN_TEST_FUNC(qemuMonitorJSONDrivePivot, "vdb")
> GEN_TEST_FUNC(qemuMonitorJSONScreendump, "devicename", 1, "/foo/bar")
> GEN_TEST_FUNC(qemuMonitorJSONOpenGraphics, "spice", "spicefd", false)
> -GEN_TEST_FUNC(qemuMonitorJSONNBDServerStart, "localhost", 12345, "test-alias")
> GEN_TEST_FUNC(qemuMonitorJSONNBDServerAdd, "vda", true)
> GEN_TEST_FUNC(qemuMonitorJSONDetachCharDev, "serial1")
> GEN_TEST_FUNC(qemuMonitorJSONBlockdevTrayOpen, "foodev", true)
> @@ -1356,6 +1355,61 @@ GEN_TEST_FUNC(qemuMonitorJSONBlockdevTrayClose, "foodev")
> GEN_TEST_FUNC(qemuMonitorJSONBlockdevMediumRemove, "foodev")
> GEN_TEST_FUNC(qemuMonitorJSONBlockdevMediumInsert, "foodev", "newnode")
>
> +static int
> +testQemuMonitorJSONqemuMonitorJSONNBDServerStart(const void *opaque)
> +{
> + virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr) opaque;
> + qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt);
I'd suggest you use qemuMonitorTestNewSchema here so that we preserve
the schema checks.
> + int ret = -1;
> + virStorageNetHostDef server_tcp = {
> + .name = (char *)"localhost",
> + .port = 12345,
> + .transport = VIR_STORAGE_NET_HOST_TRANS_TCP,
> + };
> + virStorageNetHostDef server_unix = {
> + .socket = (char *)"/tmp/sock",
> + .transport = VIR_STORAGE_NET_HOST_TRANS_UNIX,
> + };
> +
> + if (!test)
> + return -1;
> +
> + if (qemuMonitorTestAddItemVerbatim(test,
> + "{\"execute\":\"nbd-server-start\","
> + " \"arguments\":{\"addr\":{"
> + " \"type\":\"inet\",\"data\":{"
> + " \"host\":\"localhost\","
> + " \"port\":\"12345\"}},"
> + " \"tls-creds\":\"test-alias\"},"
> + " \"id\":\"libvirt-1\"}",
> + NULL, "{\"return\":{}}") < 0)
> + goto cleanup;
> +
> + if (qemuMonitorTestAddItemVerbatim(test,
> + "{\"execute\":\"nbd-server-start\","
> + " \"arguments\":{\"addr\":{"
> + " \"type\":\"unix\",\"data\":{"
> + " \"path\":\"/tmp/sock\"}},"
> + " \"tls-creds\":\"test-alias\"},"
> + " \"id\":\"libvirt-2\"}",
> + NULL, "{\"return\":{}}") < 0)
> + goto cleanup;
> +
> + if (qemuMonitorJSONNBDServerStart(qemuMonitorTestGetMonitor(test),
> + &server_tcp, "test-alias") < 0)
> + goto cleanup;
> +
> + if (qemuMonitorJSONNBDServerStart(qemuMonitorTestGetMonitor(test),
> + &server_unix, "test-alias") < 0)
> + goto cleanup;
> +
> + ret = 0;
> +
> + cleanup:
> + qemuMonitorTestFree(test);
> + return ret;
> +}
> +
> static bool
> testQemuMonitorJSONqemuMonitorJSONQueryCPUsEqual(struct qemuMonitorQueryCpusEntry *a,
> struct qemuMonitorQueryCpusEntry *b)
> @@ -3014,7 +3068,6 @@ mymain(void)
> DO_TEST_GEN(qemuMonitorJSONDrivePivot);
> DO_TEST_GEN(qemuMonitorJSONScreendump);
> DO_TEST_GEN(qemuMonitorJSONOpenGraphics);
> - DO_TEST_GEN(qemuMonitorJSONNBDServerStart);
Which are deleted here.
> DO_TEST_GEN(qemuMonitorJSONNBDServerAdd);
> DO_TEST_GEN(qemuMonitorJSONDetachCharDev);
> DO_TEST_GEN(qemuMonitorJSONBlockdevTrayOpen);
> @@ -3036,6 +3089,7 @@ mymain(void)
ACK with schema checks preserved.
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
