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

Re: [PATCH v2 6/9] qemu: Allow NBD migration over UNIX socket



On Tue, Sep 01, 2020 at 16:36:57 +0200, Martin Kletzander wrote:
> Adds new typed param for migration and uses this as a UNIX socket path that
> should be used for the NBD part of migration.  And also adds virsh support.
> 
> Partially resolves: https://bugzilla.redhat.com/1638889
> 
> Signed-off-by: Martin Kletzander <mkletzan redhat com>
> ---
>  docs/manpages/virsh.rst          |  11 ++
>  include/libvirt/libvirt-domain.h |  13 +++
>  src/qemu/qemu_driver.c           |  33 +++++-
>  src/qemu/qemu_migration.c        | 170 ++++++++++++++++++++++++-------
>  src/qemu/qemu_migration.h        |   3 +
>  src/qemu/qemu_migration_cookie.c |   3 +-
>  tools/virsh-domain.c             |  12 +++
>  7 files changed, 205 insertions(+), 40 deletions(-)
> 
> diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
> index 8e2fb7039046..12357ea4ee86 100644
> --- a/docs/manpages/virsh.rst
> +++ b/docs/manpages/virsh.rst
> @@ -3113,6 +3113,7 @@ migrate
>        [--postcopy-bandwidth bandwidth]
>        [--parallel [--parallel-connections connections]]
>        [--bandwidth bandwidth] [--tls-destination hostname]
> +      [--disks-socket socket-path]

--disks-uri URI

>  
>  Migrate domain to another host.  Add *--live* for live migration; <--p2p>
>  for peer-2-peer migration; *--direct* for direct migration; or *--tunnelled*
> @@ -3292,6 +3293,16 @@ error if this parameter is used.
>  Optional *disks-port* sets the port that hypervisor on destination side should
>  bind to for incoming disks traffic. Currently it is supported only by QEMU.
>  
> +Optional *disks-uri* can also be specified (mutually exclusive with
> +*disks-port*) to specify what the remote hypervisor should bind/connect to when
> +migrating disks.  This can be *tcp://address:port* to specify a listen address
> +(which overrides *--listen-address* for the disk migration) and a port or
> +*unix:///path/to/socket* in case you need the disk migration to happen over a
> +UNIX socket with that specified path.  In this case you need to make sure the
> +same socket path is accessible to both source and destination hypervisors and
> +connecting to the socket on the source (after hypervisor creates it on the
> +destination) will actually connect to the destination.
> +
>  
>  migrate-compcache
>  -----------------
...
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 3636716ceea1..08b6b853de47 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
...
> @@ -11485,6 +11486,7 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn,
>      g_autofree const char **migrate_disks = NULL;
>      g_autofree char *origname = NULL;
>      g_autoptr(qemuMigrationParams) migParams = NULL;
> +    const char *nbdSocketPath = NULL;

nbdURI

>  
>      virCheckFlags(QEMU_MIGRATION_FLAGS, -1);
>      if (virTypedParamsValidate(params, nparams, QEMU_MIGRATION_PARAMETERS) < 0)
> @@ -11502,6 +11504,9 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn,
>          virTypedParamsGetString(params, nparams,
>                                  VIR_MIGRATE_PARAM_LISTEN_ADDRESS,
>                                  &listenAddress) < 0 ||
> +        virTypedParamsGetString(params, nparams,
> +                                VIR_MIGRATE_PARAM_DISKS_URI,
> +                                &nbdSocketPath) < 0 ||

nbdURI

>          virTypedParamsGetInt(params, nparams,
>                               VIR_MIGRATE_PARAM_DISKS_PORT,
>                               &nbdPort) < 0)
> @@ -11518,6 +11523,13 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn,
>                                                     QEMU_MIGRATION_DESTINATION)))
>          return -1;
>  
> +    if (nbdSocketPath && nbdPort) {

nbdURI

> +        virReportError(VIR_ERR_INVALID_ARG, "%s",
> +                       _("Both port and socket requested for disk migration "

s/socket/URI/

> +                         "while being mutually exclusive"));
> +        return -1;
> +    }
> +
>      if (flags & VIR_MIGRATE_TUNNELLED) {
>          /* this is a logical error; we never should have gotten here with
>           * VIR_MIGRATE_TUNNELLED set
> @@ -11540,7 +11552,7 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn,
>                                           uri_in, uri_out,
>                                           &def, origname, listenAddress,
>                                           nmigrate_disks, migrate_disks, nbdPort,
> -                                         migParams, flags);
> +                                         nbdSocketPath, migParams, flags);

nbdURI

>  }
>  
>  
> @@ -11682,7 +11694,7 @@ qemuDomainMigratePerform3(virDomainPtr dom,
>  
>      ret = qemuMigrationSrcPerform(driver, dom->conn, vm, xmlin, NULL,
>                                    dconnuri, uri, NULL, NULL, 0, NULL, 0,
> -                                  migParams,
> +                                  NULL, migParams,
>                                    cookiein, cookieinlen,
>                                    cookieout, cookieoutlen,
>                                    flags, dname, resource, true);
> @@ -11716,6 +11728,7 @@ qemuDomainMigratePerform3Params(virDomainPtr dom,
>      unsigned long long bandwidth = 0;
>      int nbdPort = 0;
>      g_autoptr(qemuMigrationParams) migParams = NULL;
> +    const char *nbdSocketPath = NULL;

nbdURI

>      int ret = -1;
>  
>      virCheckFlags(QEMU_MIGRATION_FLAGS, -1);
> @@ -11743,11 +11756,21 @@ qemuDomainMigratePerform3Params(virDomainPtr dom,
>          virTypedParamsGetInt(params, nparams,
>                               VIR_MIGRATE_PARAM_DISKS_PORT,
>                               &nbdPort) < 0 ||
> +        virTypedParamsGetString(params, nparams,
> +                                VIR_MIGRATE_PARAM_DISKS_URI,
> +                                &nbdSocketPath) < 0 ||

nbdURI

>          virTypedParamsGetString(params, nparams,
>                                  VIR_MIGRATE_PARAM_PERSIST_XML,
>                                  &persist_xml) < 0)
>          goto cleanup;
>  
> +    if (nbdSocketPath && nbdPort) {

nbdURI

> +        virReportError(VIR_ERR_INVALID_ARG, "%s",
> +                       _("Both port and socket requested for disk migration "

s/socket/URI/

> +                         "while being mutually exclusive"));
> +        goto cleanup;
> +    }
> +
>      nmigrate_disks = virTypedParamsGetStringList(params, nparams,
>                                                   VIR_MIGRATE_PARAM_MIGRATE_DISKS,
>                                                   &migrate_disks);
> @@ -11768,7 +11791,7 @@ qemuDomainMigratePerform3Params(virDomainPtr dom,
>      ret = qemuMigrationSrcPerform(driver, dom->conn, vm, dom_xml, persist_xml,
>                                    dconnuri, uri, graphicsuri, listenAddress,
>                                    nmigrate_disks, migrate_disks, nbdPort,
> -                                  migParams,
> +                                  nbdSocketPath, migParams,

nbdURI

>                                    cookiein, cookieinlen, cookieout, cookieoutlen,
>                                    flags, dname, bandwidth, true);
>   cleanup:
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index b887185d012d..7277f2f458a2 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
...
> @@ -390,8 +391,44 @@ qemuMigrationDstStartNBDServer(virQEMUDriverPtr driver,
>          .port = nbdPort,
>      };
>      bool server_started = false;
> +    g_autoptr(virURI) uri = NULL;
> +
> +    /* Prefer nbdURI */
> +    if (nbdURI) {
> +        uri = virURIParse(nbdURI);
>  
> -    if (nbdPort < 0 || nbdPort > USHRT_MAX) {
> +        if (!uri)
> +            return -1;
> +
> +        if (STREQ(uri->scheme, "tcp")) {
> +            server.transport = VIR_STORAGE_NET_HOST_TRANS_TCP;
> +            if (!uri->server || STREQ(uri->server, "")) {
> +                /* Since tcp://:<port>/ is parsed as server = NULL and port = 0
> +                 * we should rather error out instead of auto-allocating a port
> +                 * as that would be the exact opposite of what was requested. */
> +                virReportError(VIR_ERR_INVALID_ARG,
> +                               _("URI with tcp scheme did not provide a server part: %s"),
> +                               nbdURI);
> +                return -1;
> +            }
> +                server.name = (char *)uri->server;

Indentation is a bit off here.

> +            if (uri->port)
> +                server.port = uri->port;
> +        } else if (STREQ(uri->scheme, "unix")) {
> +            if (!uri->path) {
> +                virReportError(VIR_ERR_INVALID_ARG, "%s",
> +                               _("UNIX disks URI does not include path"));
> +                return -1;
> +            }
> +            server.transport = VIR_STORAGE_NET_HOST_TRANS_UNIX;
> +            server.socket = (char *)uri->path;
> +        } else {
> +            virReportError(VIR_ERR_INVALID_ARG,
> +                           _("Unsupported scheme in disks URI: %s"),
> +                           uri->scheme);
> +            return -1;
> +        }
> +    } else if (nbdPort < 0 || nbdPort > USHRT_MAX) {
>          virReportError(VIR_ERR_INVALID_ARG, "%s",
>                         _("nbd port must be in range 0-65535"));
>          return -1;
...
> diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c
> index cef255598854..c1295b32fc27 100644
> --- a/src/qemu/qemu_migration_cookie.c
> +++ b/src/qemu/qemu_migration_cookie.c
> @@ -91,6 +91,7 @@ qemuMigrationCookieNBDFree(qemuMigrationCookieNBDPtr nbd)
>      while (nbd->ndisks)
>          VIR_FREE(nbd->disks[--nbd->ndisks].target);
>      VIR_FREE(nbd->disks);
> +
>      VIR_FREE(nbd);
>  }
>  
> @@ -992,7 +993,7 @@ qemuMigrationCookieNBDXMLParse(xmlXPathContextPtr ctxt)
>      if (port && virStrToLong_i(port, NULL, 10, &ret->port) < 0) {
>          virReportError(VIR_ERR_INTERNAL_ERROR,
>                         _("Malformed nbd port '%s'"),
> -                       port);
> +                           port);
>          goto error;
>      }
>  

I don't think you wanted to touch qemu_migration_cookie.c at all. You
can just drop both hunks.

...

With the small fixups

Reviewed-by: Jiri Denemark <jdenemar redhat com>


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