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@xxxxxxxxxx> > --- > 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@xxxxxxxxxx>
