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>