Re: [PATCH 1/4] backup: Prepare for Unix sockets in QMP nbd-server-start

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

 



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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux