[libvirt PATCH 0/9] qemu: Allow migration over UNIX sockets
KubeVirt would like to use this feature. For more information see individual commits and changes in manpages and documentation. Resolves: https://bugzilla.redhat.com/1638889 Martin Kletzander (9): qemu: Use g_autofree in qemuMigrationSrcConnect qemu: Rework qemuMigrationSrcConnect virsh: Reuse existing variable when parsing migrate --disks-port qemu: Rework starting NBD server for migration tests: Add simple test for virDomainMigrateCheckNotLocal qemu: Allow NBD migration over UNIX socket peer2peer migration: allow connecting to local sockets qemu: Allow migration over UNIX socket news: qemu: Allow migration over UNIX sockets NEWS.rst | 6 + docs/manpages/virsh.rst | 30 +++- docs/migration.html.in | 33 ++++ include/libvirt/libvirt-domain.h | 12 ++ scripts/apibuild.py | 1 + src/libvirt-domain.c | 12 +- src/libvirt_internal.h | 2 + src/libvirt_private.syms | 1 + src/qemu/qemu_domain.h | 1 + src/qemu/qemu_driver.c | 33 +++- src/qemu/qemu_migration.c | 284 ++++++++++++++++++++++--------- src/qemu/qemu_migration.h | 3 + src/qemu/qemu_migration_cookie.c | 22 ++- src/qemu/qemu_migration_cookie.h | 1 + src/remote/remote_driver.c | 8 +- src/util/viruri.c | 30 ++++ src/util/viruri.h | 2 + tests/meson.build | 1 + tests/virmigtest.c | 90 ++++++++++ tools/virsh-domain.c | 19 ++- 20 files changed, 483 insertions(+), 108 deletions(-) create mode 100644 tests/virmigtest.c -- 2.28.0
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_migration.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 142faa2cf997..60ddfde65d46 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3369,7 +3369,7 @@ qemuMigrationSrcConnect(virQEMUDriverPtr driver, { virNetSocketPtr sock; const char *host; - char *port = NULL; + g_autofree char *port = NULL; int ret = -1; host = spec->dest.host.name; @@ -3400,7 +3400,6 @@ qemuMigrationSrcConnect(virQEMUDriverPtr driver, ret = 0; cleanup: - VIR_FREE(port); if (ret < 0) VIR_FORCE_CLOSE(spec->dest.fd.qemu); return ret; -- 2.28.0
On Tue, Aug 25, 2020 at 07:47:07 +0200, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_migration.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 142faa2cf997..60ddfde65d46 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3369,7 +3369,7 @@ qemuMigrationSrcConnect(virQEMUDriverPtr driver, { virNetSocketPtr sock; const char *host; - char *port = NULL; + g_autofree char *port = NULL; int ret = -1;
host = spec->dest.host.name; @@ -3400,7 +3400,6 @@ qemuMigrationSrcConnect(virQEMUDriverPtr driver, ret = 0;
cleanup: - VIR_FREE(port); if (ret < 0) VIR_FORCE_CLOSE(spec->dest.fd.qemu); return ret;
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Instead of saving some data from a union up front and changing an overlayed struct before using said data, let's just set the new values after they are decided. This will increase the readability of future commit(s). Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_migration.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 60ddfde65d46..1a3cdb71f424 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3368,24 +3368,24 @@ qemuMigrationSrcConnect(virQEMUDriverPtr driver, qemuMigrationSpecPtr spec) { virNetSocketPtr sock; - const char *host; g_autofree char *port = NULL; + int fd_qemu = -1; int ret = -1; - host = spec->dest.host.name; - port = g_strdup_printf("%d", spec->dest.host.port); - - spec->destType = MIGRATION_DEST_FD; - spec->dest.fd.qemu = -1; - if (qemuSecuritySetSocketLabel(driver->securityManager, vm->def) < 0) goto cleanup; - if (virNetSocketNewConnectTCP(host, port, + port = g_strdup_printf("%d", spec->dest.host.port); + if (virNetSocketNewConnectTCP(spec->dest.host.name, + port, AF_UNSPEC, &sock) == 0) { - spec->dest.fd.qemu = virNetSocketDupFD(sock, true); + fd_qemu = virNetSocketDupFD(sock, true); virObjectUnref(sock); } + + spec->destType = MIGRATION_DEST_FD; + spec->dest.fd.qemu = fd_qemu; + if (qemuSecurityClearSocketLabel(driver->securityManager, vm->def) < 0 || spec->dest.fd.qemu == -1) goto cleanup; -- 2.28.0
On Tue, Aug 25, 2020 at 07:47:08 +0200, Martin Kletzander wrote:
Instead of saving some data from a union up front and changing an overlayed struct before using said data, let's just set the new values after they are decided. This will increase the readability of future commit(s).
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_migration.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- tools/virsh-domain.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 1f3a549d9ab0..c606900a9268 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -10704,7 +10704,6 @@ doMigrate(void *opaque) virDomainPtr dom = NULL; const char *desturi = NULL; const char *opt = NULL; - int disksPort = 0; unsigned int flags = 0; virshCtrlData *data = opaque; vshControl *ctl = data->ctl; @@ -10752,11 +10751,11 @@ doMigrate(void *opaque) VIR_MIGRATE_PARAM_LISTEN_ADDRESS, opt) < 0) goto save_error; - if (vshCommandOptInt(ctl, cmd, "disks-port", &disksPort) < 0) + if (vshCommandOptInt(ctl, cmd, "disks-port", &intOpt) < 0) goto out; - if (disksPort && + if (intOpt && virTypedParamsAddInt(¶ms, &nparams, &maxparams, - VIR_MIGRATE_PARAM_DISKS_PORT, disksPort) < 0) + VIR_MIGRATE_PARAM_DISKS_PORT, intOpt) < 0) goto save_error; if (vshCommandOptStringReq(ctl, cmd, "dname", &opt) < 0) -- 2.28.0
On Tue, Aug 25, 2020 at 07:47:09 +0200, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- tools/virsh-domain.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Clean up the semantics by using one extra self-describing variable. This also fixes the port allocation when the port is specified. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_migration.c | 37 +++++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 1a3cdb71f424..b887185d012d 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -383,12 +383,13 @@ qemuMigrationDstStartNBDServer(virQEMUDriverPtr driver, { int ret = -1; qemuDomainObjPrivatePtr priv = vm->privateData; - unsigned short port = 0; size_t i; virStorageNetHostDef server = { .name = (char *)listenAddr, /* cast away const */ .transport = VIR_STORAGE_NET_HOST_TRANS_TCP, + .port = nbdPort, }; + bool server_started = false; if (nbdPort < 0 || nbdPort > USHRT_MAX) { virReportError(VIR_ERR_INVALID_ARG, "%s", @@ -424,20 +425,27 @@ qemuMigrationDstStartNBDServer(virQEMUDriverPtr driver, devicename = diskAlias; } + if (!server_started) { + if (server.port) { + if (virPortAllocatorSetUsed(server.port) < 0) + goto cleanup; + } else { + unsigned short port = 0; + + if (virPortAllocatorAcquire(driver->migrationPorts, &port) < 0) + goto cleanup; + + server.port = port; + } + } + if (qemuDomainObjEnterMonitorAsync(driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN) < 0) goto cleanup; - if (port == 0) { - if (nbdPort) - port = nbdPort; - else if (virPortAllocatorAcquire(driver->migrationPorts, &port) < 0) - goto exit_monitor; - - server.port = port; - if (qemuMonitorNBDServerStart(priv->mon, &server, tls_alias) < 0) - goto exit_monitor; - } + if (!server_started && + qemuMonitorNBDServerStart(priv->mon, &server, tls_alias) < 0) + goto exit_monitor; if (qemuMonitorNBDServerAdd(priv->mon, devicename, exportname, true, NULL) < 0) goto exit_monitor; @@ -445,12 +453,13 @@ qemuMigrationDstStartNBDServer(virQEMUDriverPtr driver, goto cleanup; } - priv->nbdPort = port; + priv->nbdPort = server.port; + ret = 0; cleanup: - if (ret < 0 && nbdPort == 0) - virPortAllocatorRelease(port); + if (ret < 0) + virPortAllocatorRelease(server.port); return ret; exit_monitor: -- 2.28.0
On Tue, Aug 25, 2020 at 07:47:10 +0200, Martin Kletzander wrote:
Clean up the semantics by using one extra self-describing variable. This also fixes the port allocation when the port is specified.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_migration.c | 37 +++++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 14 deletions(-)
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
For this we need to make the function accessible (at least privately). The behaviour will change in following patches and the test helps explaining the change. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- scripts/apibuild.py | 1 + src/libvirt-domain.c | 4 +- src/libvirt_internal.h | 2 + src/libvirt_private.syms | 1 + tests/meson.build | 1 + tests/virmigtest.c | 90 ++++++++++++++++++++++++++++++++++++++++ 6 files changed, 96 insertions(+), 3 deletions(-) create mode 100644 tests/virmigtest.c diff --git a/scripts/apibuild.py b/scripts/apibuild.py index 58ae76d29cfc..b94c0f6c09dd 100755 --- a/scripts/apibuild.py +++ b/scripts/apibuild.py @@ -81,6 +81,7 @@ ignored_words = { ignored_functions = { "virConnectSupportsFeature": "private function for remote access", + "virDomainMigrateCheckNotLocal": "private function for migration", "virDomainMigrateFinish": "private function for migration", "virDomainMigrateFinish2": "private function for migration", "virDomainMigratePerform": "private function for migration", diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index ad60a92da879..4d958ca5219d 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -3269,8 +3269,7 @@ virDomainMigrateVersion3Params(virDomainPtr domain, params, nparams, true, flags); } - -static int +int virDomainMigrateCheckNotLocal(const char *dconnuri) { g_autoptr(virURI) tempuri = NULL; @@ -3286,7 +3285,6 @@ virDomainMigrateCheckNotLocal(const char *dconnuri) return 0; } - static int virDomainMigrateUnmanagedProto2(virDomainPtr domain, const char *dconnuri, diff --git a/src/libvirt_internal.h b/src/libvirt_internal.h index 72c61274a74e..cb239b3c1c44 100644 --- a/src/libvirt_internal.h +++ b/src/libvirt_internal.h @@ -129,6 +129,8 @@ typedef enum { int virConnectSupportsFeature(virConnectPtr conn, int feature); +int virDomainMigrateCheckNotLocal(const char *dconnuri); + int virDomainMigratePrepare (virConnectPtr dconn, char **cookie, int *cookielen, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index f950a681792c..2f6d67cebe07 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1481,6 +1481,7 @@ virHostdevUpdateActiveUSBDevices; virConnectSupportsFeature; virDomainMigrateBegin3; virDomainMigrateBegin3Params; +virDomainMigrateCheckNotLocal; virDomainMigrateConfirm3; virDomainMigrateConfirm3Params; virDomainMigrateFinish; diff --git a/tests/meson.build b/tests/meson.build index b5f6e2267aaf..49bcf5eb8a8d 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -331,6 +331,7 @@ tests += [ { 'name': 'virtypedparamtest' }, { 'name': 'viruritest' }, { 'name': 'vshtabletest', 'link_with': [ libvirt_shell_lib ] }, + { 'name': 'virmigtest' }, ] if host_machine.system() == 'linux' diff --git a/tests/virmigtest.c b/tests/virmigtest.c new file mode 100644 index 000000000000..9539aadb5157 --- /dev/null +++ b/tests/virmigtest.c @@ -0,0 +1,90 @@ +/* + * Copyright (C) 2020 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#include <config.h> + +#include "libvirt_internal.h" + +#include "testutils.h" +#include "virlog.h" +#include "viruri.h" + +#define VIR_FROM_THIS VIR_FROM_RPC + +VIR_LOG_INIT("tests.migtest"); + +struct MigLocalData { + const char *uri; + bool fail; +}; + +extern int virDomainMigrateCheckNotLocal(const char *dconnuri); + +static int testMigNotLocal(const void *args) +{ + int ret = -1; + const struct MigLocalData *data = args; + + ret = virDomainMigrateCheckNotLocal(data->uri); + + if (ret == -1) { + if (data->fail) { + virResetLastError(); + return 0; + } + return -1; + } + + if (data->fail) { + VIR_TEST_DEBUG("passed instead of expected failure"); + return -1; + } + + return ret; +} + +static int +mymain(void) +{ + int ret = 0; + +#define TEST_FULL(uri, fail) \ + do { \ + const struct MigLocalData data = { \ + uri, fail \ + }; \ + if (virTestRun("Test URI " # uri, testMigNotLocal, &data) < 0) \ + ret = -1; \ + } while (0) + +#define TEST(uri) TEST_FULL(uri, false) +#define TEST_FAIL(uri) TEST_FULL(uri, true) + + TEST_FAIL("qemu:///system"); + + TEST_FAIL("//localhost"); + TEST_FAIL("tcp://localhost.localdomain/"); + + TEST("scheme://some.cryptorandom.fqdn.tld"); + + TEST_FAIL("hehe+unix:///?socket=/path/to/some-sock"); + + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; +} + +VIR_TEST_MAIN(mymain) -- 2.28.0
On Tue, Aug 25, 2020 at 07:47:11 +0200, Martin Kletzander wrote:
For this we need to make the function accessible (at least privately). The behaviour will change in following patches and the test helps explaining the change.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- scripts/apibuild.py | 1 + src/libvirt-domain.c | 4 +- src/libvirt_internal.h | 2 + src/libvirt_private.syms | 1 + tests/meson.build | 1 + tests/virmigtest.c | 90 ++++++++++++++++++++++++++++++++++++++++ 6 files changed, 96 insertions(+), 3 deletions(-) create mode 100644 tests/virmigtest.c
diff --git a/scripts/apibuild.py b/scripts/apibuild.py index 58ae76d29cfc..b94c0f6c09dd 100755 --- a/scripts/apibuild.py +++ b/scripts/apibuild.py @@ -81,6 +81,7 @@ ignored_words = {
ignored_functions = { "virConnectSupportsFeature": "private function for remote access", + "virDomainMigrateCheckNotLocal": "private function for migration", "virDomainMigrateFinish": "private function for migration", "virDomainMigrateFinish2": "private function for migration", "virDomainMigratePerform": "private function for migration", diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index ad60a92da879..4d958ca5219d 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -3269,8 +3269,7 @@ virDomainMigrateVersion3Params(virDomainPtr domain, params, nparams, true, flags); }
- -static int +int virDomainMigrateCheckNotLocal(const char *dconnuri) { g_autoptr(virURI) tempuri = NULL; @@ -3286,7 +3285,6 @@ virDomainMigrateCheckNotLocal(const char *dconnuri) return 0; }
- static int virDomainMigrateUnmanagedProto2(virDomainPtr domain, const char *dconnuri,
I believe the two empty lines around virDomainMigrateCheckNotLocal should not be reduced to a single line. We tend to separate functions with two empty lines (except for some cases where nobody noticed) :-) ...
diff --git a/tests/virmigtest.c b/tests/virmigtest.c new file mode 100644 index 000000000000..9539aadb5157 --- /dev/null +++ b/tests/virmigtest.c @@ -0,0 +1,90 @@ ... +#define VIR_FROM_THIS VIR_FROM_RPC + +VIR_LOG_INIT("tests.migtest"); + +struct MigLocalData {
Eh, we never start a type with upper case.
+ const char *uri; + bool fail; +}; + +extern int virDomainMigrateCheckNotLocal(const char *dconnuri);
Hmm, why is this needed? Looks like a leftover.
+ +static int testMigNotLocal(const void *args) ^ |_________ | Break the line here ---'
+{ + int ret = -1; + const struct MigLocalData *data = args; ...
With the nits addressed: Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
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 | 8 +++ include/libvirt/libvirt-domain.h | 12 ++++ src/qemu/qemu_domain.h | 1 + src/qemu/qemu_driver.c | 33 ++++++++-- src/qemu/qemu_migration.c | 110 ++++++++++++++++++++++--------- src/qemu/qemu_migration.h | 3 + src/qemu/qemu_migration_cookie.c | 22 +++++-- src/qemu/qemu_migration_cookie.h | 1 + tools/virsh-domain.c | 12 ++++ 9 files changed, 160 insertions(+), 42 deletions(-) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index 9187037a5615..75f475eea6ad 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] 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,13 @@ 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-socket* path can also be specified (mutually exclusive with +*disks-port*) 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/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 8b9d9c110c19..b6c6b402b9b6 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -981,6 +981,18 @@ typedef enum { */ # define VIR_MIGRATE_PARAM_DISKS_PORT "disks_port" +/** + * VIR_MIGRATE_PARAM_DISKS_SOCKET: + * + * virDomainMigrate* params field: path of a UNIX socket that destination server + * should use for incoming disks migration. Type is VIR_TYPED_PARAM_STRING. If + * omitted, migration will proceed over network (default). At the moment this is + * only supported by the QEMU driver. This is only usable if the management + * application makes sure that socket created with this name on the destination + * will be reachable from the source under the same exact path. + */ +# define VIR_MIGRATE_PARAM_DISKS_SOCKET "disks_socket" + /** * VIR_MIGRATE_PARAM_COMPRESSION: * diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index adba79aded54..71a644ca6b95 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -166,6 +166,7 @@ struct _qemuDomainObjPrivate { unsigned long migMaxBandwidth; char *origname; int nbdPort; /* Port used for migration with NBD */ + char *nbdSocketPath; /* Port used for migration with NBD */ unsigned short migrationPort; int preMigrationState; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3636716ceea1..74b0348ccc4c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11233,7 +11233,7 @@ qemuDomainMigratePrepare2(virConnectPtr dconn, ret = qemuMigrationDstPrepareDirect(driver, dconn, NULL, 0, NULL, NULL, /* No cookies */ uri_in, uri_out, - &def, origname, NULL, 0, NULL, 0, + &def, origname, NULL, 0, NULL, 0, NULL, migParams, flags); cleanup: @@ -11289,6 +11289,7 @@ qemuDomainMigratePerform(virDomainPtr dom, */ ret = qemuMigrationSrcPerform(driver, dom->conn, vm, NULL, NULL, dconnuri, uri, NULL, NULL, 0, NULL, 0, + NULL, migParams, cookie, cookielen, NULL, NULL, /* No output cookies in v2 */ flags, dname, resource, false); @@ -11459,7 +11460,7 @@ qemuDomainMigratePrepare3(virConnectPtr dconn, cookieout, cookieoutlen, uri_in, uri_out, &def, origname, NULL, 0, NULL, 0, - migParams, flags); + NULL, migParams, flags); } static int @@ -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; 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_SOCKET, + &nbdSocketPath) < 0 || 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) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("Both port and socket requested for disk migration " + "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); } @@ -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; 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_SOCKET, + &nbdSocketPath) < 0 || virTypedParamsGetString(params, nparams, VIR_MIGRATE_PARAM_PERSIST_XML, &persist_xml) < 0) goto cleanup; + if (nbdSocketPath && nbdPort) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("Both port and socket requested for disk migration " + "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, 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..3f4690f8fb72 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -379,6 +379,7 @@ qemuMigrationDstStartNBDServer(virQEMUDriverPtr driver, size_t nmigrate_disks, const char **migrate_disks, int nbdPort, + const char *nbdSocketPath, const char *tls_alias) { int ret = -1; @@ -386,7 +387,7 @@ qemuMigrationDstStartNBDServer(virQEMUDriverPtr driver, size_t i; virStorageNetHostDef server = { .name = (char *)listenAddr, /* cast away const */ - .transport = VIR_STORAGE_NET_HOST_TRANS_TCP, + .transport = VIR_STORAGE_NET_HOST_TRANS_UNIX, .port = nbdPort, }; bool server_started = false; @@ -397,6 +398,13 @@ qemuMigrationDstStartNBDServer(virQEMUDriverPtr driver, return -1; } + /* Prefer nbdSocketPath as there is no way to indicate we do not want to + * listen on a port */ + if (nbdSocketPath) { + server.socket = (char *)nbdSocketPath; + server.transport = VIR_STORAGE_NET_HOST_TRANS_UNIX; + } + for (i = 0; i < vm->def->ndisks; i++) { virDomainDiskDefPtr disk = vm->def->disks[i]; g_autofree char *diskAlias = NULL; @@ -425,7 +433,7 @@ qemuMigrationDstStartNBDServer(virQEMUDriverPtr driver, devicename = diskAlias; } - if (!server_started) { + if (!server_started && !nbdSocketPath) { if (server.port) { if (virPortAllocatorSetUsed(server.port) < 0) goto cleanup; @@ -453,7 +461,10 @@ qemuMigrationDstStartNBDServer(virQEMUDriverPtr driver, goto cleanup; } - priv->nbdPort = server.port; + if (nbdSocketPath) + priv->nbdSocketPath = g_strdup(server.socket); + else + priv->nbdPort = server.port; ret = 0; @@ -793,6 +804,7 @@ static virStorageSourcePtr qemuMigrationSrcNBDStorageCopyBlockdevPrepareSource(virDomainDiskDefPtr disk, const char *host, int port, + const char *socket, const char *tlsAlias) { g_autoptr(virStorageSource) copysrc = NULL; @@ -813,9 +825,14 @@ qemuMigrationSrcNBDStorageCopyBlockdevPrepareSource(virDomainDiskDefPtr disk, copysrc->hosts = g_new0(virStorageNetHostDef, 1); copysrc->nhosts = 1; - copysrc->hosts->transport = VIR_STORAGE_NET_HOST_TRANS_TCP; - copysrc->hosts->port = port; - copysrc->hosts->name = g_strdup(host); + if (socket) { + copysrc->hosts->transport = VIR_STORAGE_NET_HOST_TRANS_UNIX; + copysrc->hosts->socket = g_strdup(socket); + } else { + copysrc->hosts->transport = VIR_STORAGE_NET_HOST_TRANS_TCP; + copysrc->hosts->port = port; + copysrc->hosts->name = g_strdup(host); + } copysrc->tlsAlias = g_strdup(tlsAlias); @@ -835,6 +852,7 @@ qemuMigrationSrcNBDStorageCopyBlockdev(virQEMUDriverPtr driver, bool persistjob, const char *host, int port, + const char *socket, unsigned long long mirror_speed, unsigned int mirror_shallow, const char *tlsAlias) @@ -846,7 +864,7 @@ qemuMigrationSrcNBDStorageCopyBlockdev(virQEMUDriverPtr driver, VIR_DEBUG("starting blockdev mirror for disk=%s to host=%s", disk->dst, host); - if (!(copysrc = qemuMigrationSrcNBDStorageCopyBlockdevPrepareSource(disk, host, port, tlsAlias))) + if (!(copysrc = qemuMigrationSrcNBDStorageCopyBlockdevPrepareSource(disk, host, port, socket, tlsAlias))) return -1; /* Migration via blockdev-mirror was supported sooner than the auto-read-only @@ -885,13 +903,17 @@ qemuMigrationSrcNBDStorageCopyDriveMirror(virQEMUDriverPtr driver, const char *diskAlias, const char *host, int port, + const char *socket, unsigned long long mirror_speed, bool mirror_shallow) { g_autofree char *nbd_dest = NULL; int mon_ret; - if (strchr(host, ':')) { + if (socket) { + nbd_dest = g_strdup_printf("nbd+unix:///%s?socket=%s", + diskAlias, socket); + } else if (strchr(host, ':')) { nbd_dest = g_strdup_printf("nbd:[%s]:%d:exportname=%s", host, port, diskAlias); } else { @@ -920,6 +942,7 @@ qemuMigrationSrcNBDStorageCopyOne(virQEMUDriverPtr driver, virDomainDiskDefPtr disk, const char *host, int port, + const char *socket, unsigned long long mirror_speed, bool mirror_shallow, const char *tlsAlias, @@ -958,13 +981,13 @@ qemuMigrationSrcNBDStorageCopyOne(virQEMUDriverPtr driver, rc = qemuMigrationSrcNBDStorageCopyBlockdev(driver, vm, disk, jobname, sourcename, persistjob, - host, port, + host, port, socket, mirror_speed, mirror_shallow, tlsAlias); } else { rc = qemuMigrationSrcNBDStorageCopyDriveMirror(driver, vm, diskAlias, - host, port, + host, port, socket, mirror_speed, mirror_shallow); } @@ -1014,6 +1037,7 @@ qemuMigrationSrcNBDStorageCopy(virQEMUDriverPtr driver, const char **migrate_disks, virConnectPtr dconn, const char *tlsAlias, + const char *socket, unsigned int flags) { qemuDomainObjPrivatePtr priv = vm->privateData; @@ -1038,6 +1062,9 @@ qemuMigrationSrcNBDStorageCopy(virQEMUDriverPtr driver, port = mig->nbd->port; mig->nbd->port = 0; + if (qemuSecurityDomainSetPathLabel(driver, vm, socket, false) < 0) + return -1; + for (i = 0; i < vm->def->ndisks; i++) { virDomainDiskDefPtr disk = vm->def->disks[i]; @@ -1046,6 +1073,7 @@ qemuMigrationSrcNBDStorageCopy(virQEMUDriverPtr driver, continue; if (qemuMigrationSrcNBDStorageCopyOne(driver, vm, disk, host, port, + socket, mirror_speed, mirror_shallow, tlsAlias, flags) < 0) return -1; @@ -2329,6 +2357,8 @@ qemuMigrationDstPrepare(virDomainObjPtr vm, if (tunnel) { migrateFrom = g_strdup("stdio"); + } else if (g_strcmp0(protocol, "unix") == 0) { + migrateFrom = g_strdup_printf("%s:%s", protocol, listenAddress); } else { bool encloseAddress = false; bool hostIPv6Capable = false; @@ -2397,6 +2427,7 @@ qemuMigrationDstPrepareAny(virQEMUDriverPtr driver, size_t nmigrate_disks, const char **migrate_disks, int nbdPort, + const char *nbdSocketPath, qemuMigrationParamsPtr migParams, unsigned long flags) { @@ -2650,7 +2681,8 @@ qemuMigrationDstPrepareAny(virQEMUDriverPtr driver, if (qemuMigrationDstStartNBDServer(driver, vm, incoming->address, nmigrate_disks, migrate_disks, - nbdPort, nbdTLSAlias) < 0) { + nbdPort, nbdSocketPath, + nbdTLSAlias) < 0) { goto stopjob; } cookieFlags |= QEMU_MIGRATION_COOKIE_NBD; @@ -2785,7 +2817,7 @@ qemuMigrationDstPrepareTunnel(virQEMUDriverPtr driver, return qemuMigrationDstPrepareAny(driver, dconn, cookiein, cookieinlen, cookieout, cookieoutlen, def, origname, st, NULL, 0, false, NULL, 0, NULL, 0, - migParams, flags); + NULL, migParams, flags); } @@ -2826,6 +2858,7 @@ qemuMigrationDstPrepareDirect(virQEMUDriverPtr driver, size_t nmigrate_disks, const char **migrate_disks, int nbdPort, + const char *nbdSocketPath, qemuMigrationParamsPtr migParams, unsigned long flags) { @@ -2840,11 +2873,13 @@ qemuMigrationDstPrepareDirect(virQEMUDriverPtr driver, VIR_DEBUG("driver=%p, dconn=%p, cookiein=%s, cookieinlen=%d, " "cookieout=%p, cookieoutlen=%p, uri_in=%s, uri_out=%p, " "def=%p, origname=%s, listenAddress=%s, " - "nmigrate_disks=%zu, migrate_disks=%p, nbdPort=%d, flags=0x%lx", + "nmigrate_disks=%zu, migrate_disks=%p, nbdPort=%d, " + "nbdSocketPath=%s, flags=0x%lx", driver, dconn, NULLSTR(cookiein), cookieinlen, cookieout, cookieoutlen, NULLSTR(uri_in), uri_out, *def, origname, NULLSTR(listenAddress), - nmigrate_disks, migrate_disks, nbdPort, flags); + nmigrate_disks, migrate_disks, nbdPort, NULLSTR(nbdSocketPath), + flags); *uri_out = NULL; @@ -2947,7 +2982,7 @@ qemuMigrationDstPrepareDirect(virQEMUDriverPtr driver, NULL, uri ? uri->scheme : "tcp", port, autoPort, listenAddress, nmigrate_disks, migrate_disks, nbdPort, - migParams, flags); + nbdSocketPath, migParams, flags); cleanup: if (ret != 0) { VIR_FREE(*uri_out); @@ -3482,7 +3517,8 @@ qemuMigrationSrcRun(virQEMUDriverPtr driver, const char *graphicsuri, size_t nmigrate_disks, const char **migrate_disks, - qemuMigrationParamsPtr migParams) + qemuMigrationParamsPtr migParams, + const char *nbdSocketPath) { int ret = -1; unsigned int migrate_flags = QEMU_MONITOR_MIGRATE_BACKGROUND; @@ -3614,7 +3650,8 @@ qemuMigrationSrcRun(virQEMUDriverPtr driver, &migrate_flags, nmigrate_disks, migrate_disks, - dconn, tlsAlias, flags) < 0) { + dconn, tlsAlias, + nbdSocketPath, flags) < 0) { goto error; } } else { @@ -3854,7 +3891,8 @@ qemuMigrationSrcPerformNative(virQEMUDriverPtr driver, const char *graphicsuri, size_t nmigrate_disks, const char **migrate_disks, - qemuMigrationParamsPtr migParams) + qemuMigrationParamsPtr migParams, + const char *nbdSocketPath) { qemuDomainObjPrivatePtr priv = vm->privateData; g_autoptr(virURI) uribits = NULL; @@ -3908,7 +3946,7 @@ qemuMigrationSrcPerformNative(virQEMUDriverPtr driver, ret = qemuMigrationSrcRun(driver, vm, persist_xml, cookiein, cookieinlen, cookieout, cookieoutlen, flags, resource, &spec, dconn, graphicsuri, nmigrate_disks, migrate_disks, - migParams); + migParams, nbdSocketPath); if (spec.destType == MIGRATION_DEST_FD) VIR_FORCE_CLOSE(spec.dest.fd.qemu); @@ -3971,7 +4009,7 @@ qemuMigrationSrcPerformTunnel(virQEMUDriverPtr driver, ret = qemuMigrationSrcRun(driver, vm, persist_xml, cookiein, cookieinlen, cookieout, cookieoutlen, flags, resource, &spec, dconn, graphicsuri, nmigrate_disks, migrate_disks, - migParams); + migParams, NULL); cleanup: VIR_FORCE_CLOSE(spec.dest.fd.qemu); @@ -4078,7 +4116,7 @@ qemuMigrationSrcPerformPeer2Peer2(virQEMUDriverPtr driver, cookie, cookielen, NULL, NULL, /* No out cookie with v2 migration */ flags, resource, dconn, NULL, 0, NULL, - migParams); + migParams, NULL); /* Perform failed. Make sure Finish doesn't overwrite the error */ if (ret < 0) @@ -4142,6 +4180,7 @@ qemuMigrationSrcPerformPeer2Peer3(virQEMUDriverPtr driver, size_t nmigrate_disks, const char **migrate_disks, int nbdPort, + const char *nbdSocketPath, qemuMigrationParamsPtr migParams, unsigned long long bandwidth, bool useParams, @@ -4226,6 +4265,11 @@ qemuMigrationSrcPerformPeer2Peer3(virQEMUDriverPtr driver, VIR_MIGRATE_PARAM_DISKS_PORT, nbdPort) < 0) goto cleanup; + if (nbdSocketPath && + virTypedParamsAddString(¶ms, &nparams, &maxparams, + VIR_MIGRATE_PARAM_DISKS_SOCKET, + nbdSocketPath) < 0) + goto cleanup; if (qemuMigrationParamsDump(migParams, ¶ms, &nparams, &maxparams, &flags) < 0) @@ -4323,7 +4367,7 @@ qemuMigrationSrcPerformPeer2Peer3(virQEMUDriverPtr driver, &cookieout, &cookieoutlen, flags, bandwidth, dconn, graphicsuri, nmigrate_disks, migrate_disks, - migParams); + migParams, nbdSocketPath); } /* Perform failed. Make sure Finish doesn't overwrite the error */ @@ -4498,6 +4542,7 @@ qemuMigrationSrcPerformPeer2Peer(virQEMUDriverPtr driver, size_t nmigrate_disks, const char **migrate_disks, int nbdPort, + const char *nbdSocketPath, qemuMigrationParamsPtr migParams, unsigned long flags, const char *dname, @@ -4618,7 +4663,7 @@ qemuMigrationSrcPerformPeer2Peer(virQEMUDriverPtr driver, ret = qemuMigrationSrcPerformPeer2Peer3(driver, sconn, dconn, dconnuri, vm, xmlin, persist_xml, dname, uri, graphicsuri, listenAddress, nmigrate_disks, migrate_disks, - nbdPort, migParams, resource, + nbdPort, nbdSocketPath, migParams, resource, useParams, flags); } else { ret = qemuMigrationSrcPerformPeer2Peer2(driver, sconn, dconn, vm, @@ -4654,6 +4699,7 @@ qemuMigrationSrcPerformJob(virQEMUDriverPtr driver, size_t nmigrate_disks, const char **migrate_disks, int nbdPort, + const char *nbdSocketPath, qemuMigrationParamsPtr migParams, const char *cookiein, int cookieinlen, @@ -4692,6 +4738,7 @@ qemuMigrationSrcPerformJob(virQEMUDriverPtr driver, ret = qemuMigrationSrcPerformPeer2Peer(driver, conn, vm, xmlin, persist_xml, dconnuri, uri, graphicsuri, listenAddress, nmigrate_disks, migrate_disks, nbdPort, + nbdSocketPath, migParams, flags, dname, resource, &v3proto); } else { @@ -4699,7 +4746,7 @@ qemuMigrationSrcPerformJob(virQEMUDriverPtr driver, ret = qemuMigrationSrcPerformNative(driver, vm, persist_xml, uri, cookiein, cookieinlen, cookieout, cookieoutlen, flags, resource, NULL, NULL, 0, NULL, - migParams); + migParams, nbdSocketPath); } if (ret < 0) goto endjob; @@ -4765,7 +4812,8 @@ qemuMigrationSrcPerformPhase(virQEMUDriverPtr driver, char **cookieout, int *cookieoutlen, unsigned long flags, - unsigned long resource) + unsigned long resource, + const char *nbdSocketPath) { qemuDomainObjPrivatePtr priv = vm->privateData; qemuDomainJobPrivatePtr jobPriv = priv->job.privateData; @@ -4787,7 +4835,7 @@ qemuMigrationSrcPerformPhase(virQEMUDriverPtr driver, ret = qemuMigrationSrcPerformNative(driver, vm, persist_xml, uri, cookiein, cookieinlen, cookieout, cookieoutlen, flags, resource, NULL, graphicsuri, - nmigrate_disks, migrate_disks, migParams); + nmigrate_disks, migrate_disks, migParams, nbdSocketPath); if (ret < 0) { qemuMigrationSrcRestoreDomainState(driver, vm); @@ -4828,6 +4876,7 @@ qemuMigrationSrcPerform(virQEMUDriverPtr driver, size_t nmigrate_disks, const char **migrate_disks, int nbdPort, + const char *nbdSocketPath, qemuMigrationParamsPtr migParams, const char *cookiein, int cookieinlen, @@ -4841,11 +4890,12 @@ qemuMigrationSrcPerform(virQEMUDriverPtr driver, VIR_DEBUG("driver=%p, conn=%p, vm=%p, xmlin=%s, dconnuri=%s, " "uri=%s, graphicsuri=%s, listenAddress=%s, " "nmigrate_disks=%zu, migrate_disks=%p, nbdPort=%d, " + "nbdSocketPath=%s, " "cookiein=%s, cookieinlen=%d, cookieout=%p, cookieoutlen=%p, " "flags=0x%lx, dname=%s, resource=%lu, v3proto=%d", driver, conn, vm, NULLSTR(xmlin), NULLSTR(dconnuri), NULLSTR(uri), NULLSTR(graphicsuri), NULLSTR(listenAddress), - nmigrate_disks, migrate_disks, nbdPort, + nmigrate_disks, migrate_disks, nbdPort, NULLSTR(nbdSocketPath), NULLSTR(cookiein), cookieinlen, cookieout, cookieoutlen, flags, NULLSTR(dname), resource, v3proto); @@ -4859,7 +4909,7 @@ qemuMigrationSrcPerform(virQEMUDriverPtr driver, return qemuMigrationSrcPerformJob(driver, conn, vm, xmlin, persist_xml, dconnuri, uri, graphicsuri, listenAddress, nmigrate_disks, migrate_disks, nbdPort, - migParams, + nbdSocketPath, migParams, cookiein, cookieinlen, cookieout, cookieoutlen, flags, dname, resource, v3proto); @@ -4877,12 +4927,12 @@ qemuMigrationSrcPerform(virQEMUDriverPtr driver, migParams, cookiein, cookieinlen, cookieout, cookieoutlen, - flags, resource); + flags, resource, nbdSocketPath); } else { return qemuMigrationSrcPerformJob(driver, conn, vm, xmlin, persist_xml, NULL, uri, graphicsuri, listenAddress, nmigrate_disks, migrate_disks, nbdPort, - migParams, + nbdSocketPath, migParams, cookiein, cookieinlen, cookieout, cookieoutlen, flags, dname, resource, v3proto); diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index b6f88d3fd9ea..f195bda3e367 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -84,6 +84,7 @@ VIR_MIGRATE_PARAM_BANDWIDTH_POSTCOPY, VIR_TYPED_PARAM_ULLONG, \ VIR_MIGRATE_PARAM_PARALLEL_CONNECTIONS, VIR_TYPED_PARAM_INT, \ VIR_MIGRATE_PARAM_TLS_DESTINATION, VIR_TYPED_PARAM_STRING, \ + VIR_MIGRATE_PARAM_DISKS_SOCKET, VIR_TYPED_PARAM_STRING, \ NULL @@ -149,6 +150,7 @@ qemuMigrationDstPrepareDirect(virQEMUDriverPtr driver, size_t nmigrate_disks, const char **migrate_disks, int nbdPort, + const char *nbdSocketPath, qemuMigrationParamsPtr migParams, unsigned long flags); @@ -165,6 +167,7 @@ qemuMigrationSrcPerform(virQEMUDriverPtr driver, size_t nmigrate_disks, const char **migrate_disks, int nbdPort, + const char *nbdSocketPath, qemuMigrationParamsPtr migParams, const char *cookiein, int cookieinlen, diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c index 81b557e0a8b4..23511bed67d5 100644 --- a/src/qemu/qemu_migration_cookie.c +++ b/src/qemu/qemu_migration_cookie.c @@ -91,6 +91,8 @@ qemuMigrationCookieNBDFree(qemuMigrationCookieNBDPtr nbd) while (nbd->ndisks) VIR_FREE(nbd->disks[--nbd->ndisks].target); VIR_FREE(nbd->disks); + + VIR_FREE(nbd->socketPath); VIR_FREE(nbd); } @@ -464,7 +466,10 @@ qemuMigrationCookieAddNBD(qemuMigrationCookiePtr mig, mig->nbd = g_new0(qemuMigrationCookieNBD, 1); - mig->nbd->port = priv->nbdPort; + if (priv->nbdSocketPath) + mig->nbd->socketPath = priv->nbdSocketPath; + else + mig->nbd->port = priv->nbdPort; mig->flags |= QEMU_MIGRATION_COOKIE_NBD; if (vm->def->ndisks == 0) @@ -988,12 +993,15 @@ qemuMigrationCookieNBDXMLParse(xmlXPathContextPtr ctxt) if (VIR_ALLOC(ret) < 0) goto error; - port = virXPathString("string(./nbd/@port)", ctxt); - if (port && virStrToLong_i(port, NULL, 10, &ret->port) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Malformed nbd port '%s'"), - port); - goto error; + ret->socketPath = virXPathString("string(./nbd/@socketPath)", ctxt); + if (!ret->socketPath) { + port = virXPathString("string(./nbd/@port)", ctxt); + if (port && virStrToLong_i(port, NULL, 10, &ret->port) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Malformed nbd port '%s'"), + port); + goto error; + } } /* Now check if source sent a list of disks to prealloc. We might be diff --git a/src/qemu/qemu_migration_cookie.h b/src/qemu/qemu_migration_cookie.h index 95e7edb89914..90a320a69c0c 100644 --- a/src/qemu/qemu_migration_cookie.h +++ b/src/qemu/qemu_migration_cookie.h @@ -93,6 +93,7 @@ typedef struct _qemuMigrationCookieNBD qemuMigrationCookieNBD; typedef qemuMigrationCookieNBD *qemuMigrationCookieNBDPtr; struct _qemuMigrationCookieNBD { int port; /* on which port does NBD server listen for incoming data */ + char *socketPath; size_t ndisks; /* Number of items in @disk array */ struct qemuMigrationCookieNBDDisk *disks; diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index c606900a9268..20b8ab9837fd 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -10639,6 +10639,10 @@ static const vshCmdOptDef opts_migrate[] = { .type = VSH_OT_INT, .help = N_("port to use by target server for incoming disks migration") }, + {.name = "disks-socket", + .type = VSH_OT_STRING, + .help = N_("UNIX socket path to use for disks migration") + }, {.name = "comp-methods", .type = VSH_OT_STRING, .help = N_("comma separated list of compression methods to be used") @@ -10758,6 +10762,14 @@ doMigrate(void *opaque) VIR_MIGRATE_PARAM_DISKS_PORT, intOpt) < 0) goto save_error; + if (vshCommandOptStringReq(ctl, cmd, "disks-socket", &opt) < 0) + goto out; + if (opt && + virTypedParamsAddString(¶ms, &nparams, &maxparams, + VIR_MIGRATE_PARAM_DISKS_SOCKET, + opt) < 0) + goto save_error; + if (vshCommandOptStringReq(ctl, cmd, "dname", &opt) < 0) goto out; if (opt && -- 2.28.0
On Tue, Aug 25, 2020 at 07:47:12 +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 | 8 +++ include/libvirt/libvirt-domain.h | 12 ++++ src/qemu/qemu_domain.h | 1 + src/qemu/qemu_driver.c | 33 ++++++++-- src/qemu/qemu_migration.c | 110 ++++++++++++++++++++++--------- src/qemu/qemu_migration.h | 3 + src/qemu/qemu_migration_cookie.c | 22 +++++-- src/qemu/qemu_migration_cookie.h | 1 + tools/virsh-domain.c | 12 ++++ 9 files changed, 160 insertions(+), 42 deletions(-) ... diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index adba79aded54..71a644ca6b95 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -166,6 +166,7 @@ struct _qemuDomainObjPrivate { unsigned long migMaxBandwidth; char *origname; int nbdPort; /* Port used for migration with NBD */ + char *nbdSocketPath; /* Port used for migration with NBD */
Copy&pasta error :-) s/Port/Socket/
unsigned short migrationPort; int preMigrationState;
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index b887185d012d..3f4690f8fb72 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -379,6 +379,7 @@ qemuMigrationDstStartNBDServer(virQEMUDriverPtr driver, size_t nmigrate_disks, const char **migrate_disks, int nbdPort, + const char *nbdSocketPath, const char *tls_alias) { int ret = -1; @@ -386,7 +387,7 @@ qemuMigrationDstStartNBDServer(virQEMUDriverPtr driver, size_t i; virStorageNetHostDef server = { .name = (char *)listenAddr, /* cast away const */ - .transport = VIR_STORAGE_NET_HOST_TRANS_TCP, + .transport = VIR_STORAGE_NET_HOST_TRANS_UNIX,
I'd expect transport to remain VIR_STORAGE_NET_HOST_TRANS_TCP unless nbdSocketPath is specified. In other words, the following hunk should be sufficient.
.port = nbdPort, }; bool server_started = false; @@ -397,6 +398,13 @@ qemuMigrationDstStartNBDServer(virQEMUDriverPtr driver, return -1; }
+ /* Prefer nbdSocketPath as there is no way to indicate we do not want to + * listen on a port */ + if (nbdSocketPath) { + server.socket = (char *)nbdSocketPath; + server.transport = VIR_STORAGE_NET_HOST_TRANS_UNIX; + } + for (i = 0; i < vm->def->ndisks; i++) { virDomainDiskDefPtr disk = vm->def->disks[i]; g_autofree char *diskAlias = NULL; @@ -425,7 +433,7 @@ qemuMigrationDstStartNBDServer(virQEMUDriverPtr driver, devicename = diskAlias; }
- if (!server_started) { + if (!server_started && !nbdSocketPath) {
I'd probably change this to if (!server_started && server.transport == VIR_STORAGE_NET_HOST_TRANS_TCP) { to make it more obvious we only care about port allocation for TCP.
if (server.port) { if (virPortAllocatorSetUsed(server.port) < 0) goto cleanup;
...
@@ -885,13 +903,17 @@ qemuMigrationSrcNBDStorageCopyDriveMirror(virQEMUDriverPtr driver, const char *diskAlias, const char *host, int port, + const char *socket, unsigned long long mirror_speed, bool mirror_shallow) { g_autofree char *nbd_dest = NULL; int mon_ret;
- if (strchr(host, ':')) { + if (socket) { + nbd_dest = g_strdup_printf("nbd+unix:///%s?socket=%s", + diskAlias, socket);
Is the number of slashes correct here? The "nbd+unix://" is clear, but then we pass "/diskAlias" rather than just "diskAlias". Oh I see, since "//" is present, the path has to either be empty or start with "/" to form a valid URI. Everything seems to be correct then, although a bit confusing since diskAlias is not actually a path. But I guess that's what QEMU expects.
+ } else if (strchr(host, ':')) { nbd_dest = g_strdup_printf("nbd:[%s]:%d:exportname=%s", host, port, diskAlias); } else { ... @@ -2329,6 +2357,8 @@ qemuMigrationDstPrepare(virDomainObjPtr vm,
if (tunnel) { migrateFrom = g_strdup("stdio"); + } else if (g_strcmp0(protocol, "unix") == 0) { + migrateFrom = g_strdup_printf("%s:%s", protocol, listenAddress); } else { bool encloseAddress = false; bool hostIPv6Capable = false;
Looks like this hunk would better fit in 8/9 (qemu: Allow migration over UNIX socket). ... With the small issues addressed and if all I said is correct Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
On Tue, Aug 25, 2020 at 07:47:12AM +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 | 8 +++ include/libvirt/libvirt-domain.h | 12 ++++ src/qemu/qemu_domain.h | 1 + src/qemu/qemu_driver.c | 33 ++++++++-- src/qemu/qemu_migration.c | 110 ++++++++++++++++++++++--------- src/qemu/qemu_migration.h | 3 + src/qemu/qemu_migration_cookie.c | 22 +++++-- src/qemu/qemu_migration_cookie.h | 1 + tools/virsh-domain.c | 12 ++++ 9 files changed, 160 insertions(+), 42 deletions(-)
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index 9187037a5615..75f475eea6ad 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]
For the primary migration connection we have a proper URI, so we can support new schemes without adding new parameters for each one. For the NBD connetion we have the "disk port" parameter only, not a full URI. Adding "disks socket" makes this design mistake worse. IMHO we should add a "disks uri" accepting the exact same syntax as the migration URI. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Tue, Aug 25, 2020 at 16:28:12 +0100, Daniel P. Berrangé wrote:
On Tue, Aug 25, 2020 at 07:47:12AM +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 | 8 +++ include/libvirt/libvirt-domain.h | 12 ++++ src/qemu/qemu_domain.h | 1 + src/qemu/qemu_driver.c | 33 ++++++++-- src/qemu/qemu_migration.c | 110 ++++++++++++++++++++++--------- src/qemu/qemu_migration.h | 3 + src/qemu/qemu_migration_cookie.c | 22 +++++-- src/qemu/qemu_migration_cookie.h | 1 + tools/virsh-domain.c | 12 ++++ 9 files changed, 160 insertions(+), 42 deletions(-)
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index 9187037a5615..75f475eea6ad 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]
For the primary migration connection we have a proper URI, so we can support new schemes without adding new parameters for each one.
For the NBD connetion we have the "disk port" parameter only, not a full URI.
Adding "disks socket" makes this design mistake worse.
IMHO we should add a "disks uri" accepting the exact same syntax as the migration URI.
Oh yeah, thanks for mentioning this. I originally thought about generic disks-uri too, but forgot about it when getting back to the review after lunch :-/ Just beware the migration URI is basically passed through to QEMU, while we use nbd or nbd+unix URIs for disks when talking to QEMU and we should not use these in the API. Just tcp:// or unix:// and translate them to the correct QEMU nbd URI internally. Jirka
On Tue, Aug 25, 2020 at 06:01:28PM +0200, Jiri Denemark wrote:
On Tue, Aug 25, 2020 at 16:28:12 +0100, Daniel P. Berrangé wrote:
On Tue, Aug 25, 2020 at 07:47:12AM +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 | 8 +++ include/libvirt/libvirt-domain.h | 12 ++++ src/qemu/qemu_domain.h | 1 + src/qemu/qemu_driver.c | 33 ++++++++-- src/qemu/qemu_migration.c | 110 ++++++++++++++++++++++--------- src/qemu/qemu_migration.h | 3 + src/qemu/qemu_migration_cookie.c | 22 +++++-- src/qemu/qemu_migration_cookie.h | 1 + tools/virsh-domain.c | 12 ++++ 9 files changed, 160 insertions(+), 42 deletions(-)
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index 9187037a5615..75f475eea6ad 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]
For the primary migration connection we have a proper URI, so we can support new schemes without adding new parameters for each one.
For the NBD connetion we have the "disk port" parameter only, not a full URI.
Adding "disks socket" makes this design mistake worse.
IMHO we should add a "disks uri" accepting the exact same syntax as the migration URI.
Oh yeah, thanks for mentioning this. I originally thought about generic disks-uri too, but forgot about it when getting back to the review after lunch :-/
I wanted to do it that way, but I did not want to be arguing for that. So here's what I thought others would use to argue against the URI: If we allow URI it can then be used for non-socket connections as well, but in that case we might need different IP address for source and destination, the same reason we have listenAddress for. I guess my counter-argument would be that we already have the listenAddress for that. That might have been countered by saying that someone might want to migrate the disk over different data path. At that point I might say to forward it yourself using a proxy. I have few more following arguments in my head. I'm very much fine with adding URI instead, but I am not going to be arguing for either.
Just beware the migration URI is basically passed through to QEMU, while we use nbd or nbd+unix URIs for disks when talking to QEMU and we should not use these in the API. Just tcp:// or unix:// and translate them to the correct QEMU nbd URI internally.
yeah, well, qemu should accept both nbd+unix and unix: I think. I'll see what I can do.
Jirka
Local socket connections were outright disabled because there was no "server" part in the URI. However, given how requirements and usage scenarios are evolving, some management apps might need the source libvirt daemon to connect to the destination daemon over a UNIX socket for peer2peer migration. Since we cannot know where the socket leads (whether the same daemon or not) let's decide that based on whether the socket path is non-standard, or rather explicitly specified in the URI. Checking non-standard path would require to ask the daemon for configuration and the only misuse that it would prevent would be a pretty weird one. And that's not worth it. The assumption is that whenever someone uses explicit UNIX socket paths in the URI for migration they better know what they are doing. Partially resolves: https://bugzilla.redhat.com/1638889 Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- docs/manpages/virsh.rst | 9 +++++++++ src/libvirt-domain.c | 8 +++++++- src/remote/remote_driver.c | 8 ++++++-- src/util/viruri.c | 30 ++++++++++++++++++++++++++++++ src/util/viruri.h | 2 ++ tests/virmigtest.c | 2 +- 6 files changed, 55 insertions(+), 4 deletions(-) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index 75f475eea6ad..cbb3c18deb30 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -3235,6 +3235,15 @@ has different semantics: * peer2peer migration: the *desturi* is an address of the target host as seen from the source machine. +In a special circumstance where you require a complete control of the connection +and/or libvirt does not have network access to the remote side you can use a +unix transport in the URI and specify a socket path in the query, for example +with the qemu driver you could use this: + +.. code-block:: + + qemu+unix://?socket=/path/to/socket + When *migrateuri* is not specified, libvirt will automatically determine the hypervisor specific URI. Some hypervisors, including QEMU, have an optional "migration_host" configuration parameter (useful when the host has multiple diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 4d958ca5219d..fba4302e3d00 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -3276,7 +3276,13 @@ virDomainMigrateCheckNotLocal(const char *dconnuri) if (!(tempuri = virURIParse(dconnuri))) return -1; - if (!tempuri->server || STRPREFIX(tempuri->server, "localhost")) { + + /* + * If someone migrates explicitly to a unix socket, then they have to know + * what they are doing and it most probably was not a mistake. + */ + if ((tempuri->server && STRPREFIX(tempuri->server, "localhost")) || + (!tempuri->server && !virURICheckProxied(tempuri))) { virReportInvalidArg(dconnuri, "%s", _("Attempt to migrate guest to the same host")); return -1; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 0331060a2d5d..77a1c00c63a5 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -1430,9 +1430,13 @@ remoteConnectOpen(virConnectPtr conn, /* If there's a driver registered we must defer to that. * If there isn't a driver, we must connect in "direct" - * mode - see doRemoteOpen */ + * mode - see doRemoteOpen. + * One exception is if we are trying to connect to an + * unknown socket path as that might be proxied to remote + * host */ if (!conn->uri->server && - virHasDriverForURIScheme(driver)) { + virHasDriverForURIScheme(driver) && + !virURICheckProxied(conn->uri)) { ret = VIR_DRV_OPEN_DECLINED; goto cleanup; } diff --git a/src/util/viruri.c b/src/util/viruri.c index 0112186fdbc4..91f86de19a8e 100644 --- a/src/util/viruri.c +++ b/src/util/viruri.c @@ -393,3 +393,33 @@ virURIGetParam(virURIPtr uri, const char *name) _("Missing URI parameter '%s'"), name); return NULL; } + + +/** + * virCheckURIProxied: + * @uri: URI to check + * + * Check if the URI looks like it refers to a non-standard socket path. In such + * scenario the socket might be proxied to a remote server even though the URI + * looks like it is only local. + * + * Returns: true if the URI might be proxied to a remote server + */ +bool +virURICheckProxied(virURIPtr uri) +{ + size_t i = 0; + + if (!uri->scheme) + return false; + + if (STRNEQ_NULLABLE(strchr(uri->scheme, '+'), "+unix")) + return false; + + for (i = 0; i < uri->paramsCount; i++) { + if (STREQ(uri->params[i].name, "socket")) + return true; + } + + return false; +} diff --git a/src/util/viruri.h b/src/util/viruri.h index e607ecc109e7..b71f5501df07 100644 --- a/src/util/viruri.h +++ b/src/util/viruri.h @@ -62,4 +62,6 @@ int virURIResolveAlias(virConfPtr conf, const char *alias, char **uri); const char *virURIGetParam(virURIPtr uri, const char *name); +bool virURICheckProxied(virURIPtr uri); + #define VIR_URI_SERVER(uri) ((uri) && (uri)->server ? (uri)->server : "localhost") diff --git a/tests/virmigtest.c b/tests/virmigtest.c index 9539aadb5157..5f52beab1421 100644 --- a/tests/virmigtest.c +++ b/tests/virmigtest.c @@ -82,7 +82,7 @@ mymain(void) TEST("scheme://some.cryptorandom.fqdn.tld"); - TEST_FAIL("hehe+unix:///?socket=/path/to/some-sock"); + TEST("hehe+unix:///?socket=/path/to/some-sock"); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.28.0
On Tue, Aug 25, 2020 at 07:47:13 +0200, Martin Kletzander wrote:
Local socket connections were outright disabled because there was no "server" part in the URI. However, given how requirements and usage scenarios are evolving, some management apps might need the source libvirt daemon to connect to the destination daemon over a UNIX socket for peer2peer migration. Since we cannot know where the socket leads (whether the same daemon or not) let's decide that based on whether the socket path is non-standard, or rather explicitly specified in the URI. Checking non-standard path would require to ask the daemon for configuration and the only misuse that it would prevent would be a pretty weird one. And that's not worth it. The assumption is that whenever someone uses explicit UNIX socket paths in the URI for migration they better know what they are doing.
Partially resolves: https://bugzilla.redhat.com/1638889
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- docs/manpages/virsh.rst | 9 +++++++++ src/libvirt-domain.c | 8 +++++++- src/remote/remote_driver.c | 8 ++++++-- src/util/viruri.c | 30 ++++++++++++++++++++++++++++++ src/util/viruri.h | 2 ++ tests/virmigtest.c | 2 +- 6 files changed, 55 insertions(+), 4 deletions(-) ... diff --git a/src/util/viruri.c b/src/util/viruri.c index 0112186fdbc4..91f86de19a8e 100644 --- a/src/util/viruri.c +++ b/src/util/viruri.c @@ -393,3 +393,33 @@ virURIGetParam(virURIPtr uri, const char *name) _("Missing URI parameter '%s'"), name); return NULL; } + + +/** + * virCheckURIProxied: + * @uri: URI to check + * + * Check if the URI looks like it refers to a non-standard socket path. In such + * scenario the socket might be proxied to a remote server even though the URI + * looks like it is only local. + * + * Returns: true if the URI might be proxied to a remote server + */ +bool +virURICheckProxied(virURIPtr uri)
I'd call this function virURICheckUnixSocket or similar as that's what it's actually doing. It doesn't really care whether the socket is connected to a proxy or not.
+{ + size_t i = 0; + + if (!uri->scheme) + return false; + + if (STRNEQ_NULLABLE(strchr(uri->scheme, '+'), "+unix")) + return false; + + for (i = 0; i < uri->paramsCount; i++) { + if (STREQ(uri->params[i].name, "socket")) + return true; + } + + return false; +}
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
This allows: a) migration without access to network b) complete control of the migration stream c) easy migration between containerised libvirt daemons on the same host Resolves: https://bugzilla.redhat.com/1638889 Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- docs/manpages/virsh.rst | 17 ++++- docs/migration.html.in | 33 ++++++++++ src/qemu/qemu_migration.c | 128 +++++++++++++++++++++++++++----------- 3 files changed, 139 insertions(+), 39 deletions(-) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index cbb3c18deb30..82f7c9f77488 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -3237,12 +3237,12 @@ has different semantics: In a special circumstance where you require a complete control of the connection and/or libvirt does not have network access to the remote side you can use a -unix transport in the URI and specify a socket path in the query, for example +UNIX transport in the URI and specify a socket path in the query, for example with the qemu driver you could use this: .. code-block:: - qemu+unix://?socket=/path/to/socket + qemu+unix:///system?socket=/path/to/socket When *migrateuri* is not specified, libvirt will automatically determine the hypervisor specific URI. Some hypervisors, including QEMU, have an optional @@ -3270,6 +3270,14 @@ There are a few scenarios where specifying *migrateuri* may help: might be specified to choose a specific port number outside the default range in order to comply with local firewall policies. +* The *desturi* uses UNIX transport method. In this advanced case libvirt + should not guess a *migrateuri* and it should be specified using + UNIX socket path URI: + +.. code-block:: + + unix://?socket=/path/to/socket + See `https://libvirt.org/migration.html#uris <https://libvirt.org/migration.html#uris>`_ for more details on migration URIs. @@ -3296,7 +3304,10 @@ specific parameters separated by '&'. Currently recognized parameters are Optional *listen-address* sets the listen address that hypervisor on the destination side should bind to for incoming migration. Both IPv4 and IPv6 addresses are accepted as well as hostnames (the resolving is done on -destination). Some hypervisors do not support this feature and will return an +destination). In niche scenarios you can also use UNIX socket to make the +hypervisor connection over UNIX socket in which case you must make sure the +source can connect to the destination using the socket path provided by you. +Some hypervisors do not support specifying the listen address and will return an error if this parameter is used. Optional *disks-port* sets the port that hypervisor on destination side should diff --git a/docs/migration.html.in b/docs/migration.html.in index e95ee9de6f1b..9c8417674b22 100644 --- a/docs/migration.html.in +++ b/docs/migration.html.in @@ -201,6 +201,9 @@ numbers. In the latter case the management application may wish to choose a specific port number outside the default range in order to comply with local firewall policies.</li> + <li>The second URI uses UNIX transport method. In this advanced case + libvirt should not guess a *migrateuri* and it should be specified using + UNIX socket path URI: <code>unix://?socket=/path/to/socket</code>.</li> </ol> <h2><a id="config">Configuration file handling</a></h2> @@ -628,5 +631,35 @@ virsh migrate --p2p --tunnelled web1 qemu+ssh://desthost/system qemu+ssh://10.0. Supported by QEMU driver </p> + + <h3><a id="scenariounixsocket">Migration using only UNIX sockets</a></h3> + + <p> + In a niche scenarion where libvirt daemon does not have access to the + network (e.g. running in a restricted container on a host that has + accessible network), when a management application wants to have complete + control over the transfer or when migrating between two containers on the + same host all the communication can be done using UNIX sockets. This + includes connecting to non-standard socket path for the destination + daemon, using UNIX sockets for hypervisor's communication or for the NBD + data transfer. All of that can be used with both peer2peer and direct + migration options. + </p> + + <p> + Example using <code>/tmp/migdir</code> as a directory representing the + same path visible from both libvirt daemons. That can be achieved by + bind-mounting the same directory to different containers running separate + daemons or forwarding connections to these sockets manually + (using <code>socat</code>, <code>netcat</code> or a custom piece of + software): + <pre> +virsh migrate web1 [--p2p] --copy-storage-all 'qemu+unix:///system?socket=/tmp/migdir/test-sock-driver' 'unix://?socket=/tmp/migdir/test-sock-qemu' [--listen-address /tmp/migdir/test-sock-qemu] --disks-socket /tmp/migdir/test-sock-nbd + </pre> + + <p> + Supported by QEMU driver + </p> + </body> </html> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 3f4690f8fb72..1158d152869c 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2943,34 +2943,41 @@ qemuMigrationDstPrepareDirect(virQEMUDriverPtr driver, } if (STRNEQ(uri->scheme, "tcp") && - STRNEQ(uri->scheme, "rdma")) { + STRNEQ(uri->scheme, "rdma") && + STRNEQ(uri->scheme, "unix")) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, _("unsupported scheme %s in migration URI %s"), uri->scheme, uri_in); goto cleanup; } - if (uri->server == NULL) { - virReportError(VIR_ERR_INVALID_ARG, _("missing host in migration" - " URI: %s"), uri_in); - goto cleanup; - } - - if (uri->port == 0) { - if (virPortAllocatorAcquire(driver->migrationPorts, &port) < 0) + if (STREQ(uri->scheme, "unix")) { + autoPort = false; + if (!listenAddress) + listenAddress = virURIGetParam(uri, "socket"); + } else { + if (uri->server == NULL) { + virReportError(VIR_ERR_INVALID_ARG, _("missing host in migration" + " URI: %s"), uri_in); goto cleanup; + } - /* Send well-formed URI only if uri_in was well-formed */ - if (well_formed_uri) { - uri->port = port; - if (!(*uri_out = virURIFormat(uri))) + if (uri->port == 0) { + if (virPortAllocatorAcquire(driver->migrationPorts, &port) < 0) goto cleanup; + + /* Send well-formed URI only if uri_in was well-formed */ + if (well_formed_uri) { + uri->port = port; + if (!(*uri_out = virURIFormat(uri))) + goto cleanup; + } else { + *uri_out = g_strdup_printf("%s:%d", uri_in, port); + } } else { - *uri_out = g_strdup_printf("%s:%d", uri_in, port); + port = uri->port; + autoPort = false; } - } else { - port = uri->port; - autoPort = false; } } @@ -3185,6 +3192,7 @@ qemuMigrationSrcConfirm(virQEMUDriverPtr driver, enum qemuMigrationDestinationType { MIGRATION_DEST_HOST, MIGRATION_DEST_CONNECT_HOST, + MIGRATION_DEST_CONNECT_SOCKET, MIGRATION_DEST_FD, }; @@ -3204,6 +3212,10 @@ struct _qemuMigrationSpec { int port; } host; + struct { + const char *path; + } socket; + struct { int qemu; int local; @@ -3418,13 +3430,29 @@ qemuMigrationSrcConnect(virQEMUDriverPtr driver, if (qemuSecuritySetSocketLabel(driver->securityManager, vm->def) < 0) goto cleanup; - port = g_strdup_printf("%d", spec->dest.host.port); - if (virNetSocketNewConnectTCP(spec->dest.host.name, - port, - AF_UNSPEC, - &sock) == 0) { - fd_qemu = virNetSocketDupFD(sock, true); - virObjectUnref(sock); + + switch (spec->destType) { + case MIGRATION_DEST_CONNECT_HOST: + port = g_strdup_printf("%d", spec->dest.host.port); + if (virNetSocketNewConnectTCP(spec->dest.host.name, + port, + AF_UNSPEC, + &sock) == 0) { + fd_qemu = virNetSocketDupFD(sock, true); + virObjectUnref(sock); + } + break; + case MIGRATION_DEST_CONNECT_SOCKET: + if (virNetSocketNewConnectUNIX(spec->dest.socket.path, + false, NULL, + &sock) == 0) { + fd_qemu = virNetSocketDupFD(sock, true); + virObjectUnref(sock); + } + break; + case MIGRATION_DEST_HOST: + case MIGRATION_DEST_FD: + break; } spec->destType = MIGRATION_DEST_FD; @@ -3632,6 +3660,14 @@ qemuMigrationSrcRun(virQEMUDriverPtr driver, if (migrate_flags & (QEMU_MONITOR_MIGRATE_NON_SHARED_DISK | QEMU_MONITOR_MIGRATE_NON_SHARED_INC)) { if (mig->nbd) { + const char *host = ""; + + if (spec->destType == MIGRATION_DEST_HOST || + spec->destType == MIGRATION_DEST_CONNECT_HOST) { + host = spec->dest.host.name; + } + + /* Currently libvirt does not support setting up of the NBD * non-shared storage migration with TLS. As we need to honour the * VIR_MIGRATE_TLS flag, we need to reject such migration until @@ -3645,7 +3681,7 @@ qemuMigrationSrcRun(virQEMUDriverPtr driver, /* This will update migrate_flags on success */ if (qemuMigrationSrcNBDStorageCopy(driver, vm, mig, - spec->dest.host.name, + host, migrate_speed, &migrate_flags, nmigrate_disks, @@ -3693,7 +3729,8 @@ qemuMigrationSrcRun(virQEMUDriverPtr driver, goto exit_monitor; /* connect to the destination qemu if needed */ - if (spec->destType == MIGRATION_DEST_CONNECT_HOST && + if ((spec->destType == MIGRATION_DEST_CONNECT_HOST || + spec->destType == MIGRATION_DEST_CONNECT_SOCKET) && qemuMigrationSrcConnect(driver, vm, spec) < 0) { goto exit_monitor; } @@ -3716,6 +3753,7 @@ qemuMigrationSrcRun(virQEMUDriverPtr driver, break; case MIGRATION_DEST_CONNECT_HOST: + case MIGRATION_DEST_CONNECT_SOCKET: /* handled above and transformed into MIGRATION_DEST_FD */ break; @@ -3931,16 +3969,34 @@ qemuMigrationSrcPerformNative(virQEMUDriverPtr driver, } } - /* RDMA and multi-fd migration requires QEMU to connect to the destination - * itself. - */ - if (STREQ(uribits->scheme, "rdma") || (flags & VIR_MIGRATE_PARALLEL)) - spec.destType = MIGRATION_DEST_HOST; - else - spec.destType = MIGRATION_DEST_CONNECT_HOST; - spec.dest.host.protocol = uribits->scheme; - spec.dest.host.name = uribits->server; - spec.dest.host.port = uribits->port; + if (STREQ(uribits->scheme, "unix")) { + if (flags & VIR_MIGRATE_TLS) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("Migration over UNIX socket with TLS is not supported")); + return -1; + } + if (flags & VIR_MIGRATE_PARALLEL) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("Parallel migration over UNIX socket is not supported")); + return -1; + } + + spec.destType = MIGRATION_DEST_CONNECT_SOCKET; + spec.dest.socket.path = virURIGetParam(uribits, "socket"); + } else { + /* RDMA and multi-fd migration requires QEMU to connect to the destination + * itself. + */ + if (STREQ(uribits->scheme, "rdma") || (flags & VIR_MIGRATE_PARALLEL)) + spec.destType = MIGRATION_DEST_HOST; + else + spec.destType = MIGRATION_DEST_CONNECT_HOST; + + spec.dest.host.protocol = uribits->scheme; + spec.dest.host.name = uribits->server; + spec.dest.host.port = uribits->port; + } + spec.fwdType = MIGRATION_FWD_DIRECT; ret = qemuMigrationSrcRun(driver, vm, persist_xml, cookiein, cookieinlen, cookieout, -- 2.28.0
On Tue, Aug 25, 2020 at 07:47:14AM +0200, Martin Kletzander wrote:
This allows:
a) migration without access to network
b) complete control of the migration stream
c) easy migration between containerised libvirt daemons on the same host
Resolves: https://bugzilla.redhat.com/1638889
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- docs/manpages/virsh.rst | 17 ++++- docs/migration.html.in | 33 ++++++++++ src/qemu/qemu_migration.c | 128 +++++++++++++++++++++++++++-----------
+virsh migrate web1 [--p2p] --copy-storage-all 'qemu+unix:///system?socket=/tmp/migdir/test-sock-driver' 'unix://?socket=/tmp/migdir/test-sock-qemu' [--listen-address /tmp/migdir/test-sock-qemu] --disks-socket /tmp/migdir/test-sock-nbd + </pre>
Why unix://?socket=/tmp/migdir/test-sock-qemu instead of the more obvious unix:///tmp/migdir/test-sock-qemu ? It feels like ?socket is just reinventing the URI path component. That ?socket syntax makes sense for the libvirtd URIs, because we use the path component to identify the libvirtd instance. For the migration URI nothing is using the path component, so we should use that for the UNIX path. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Tue, Aug 25, 2020 at 04:31:29PM +0100, Daniel P. Berrangé wrote:
On Tue, Aug 25, 2020 at 07:47:14AM +0200, Martin Kletzander wrote:
This allows:
a) migration without access to network
b) complete control of the migration stream
c) easy migration between containerised libvirt daemons on the same host
Resolves: https://bugzilla.redhat.com/1638889
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- docs/manpages/virsh.rst | 17 ++++- docs/migration.html.in | 33 ++++++++++ src/qemu/qemu_migration.c | 128 +++++++++++++++++++++++++++-----------
+virsh migrate web1 [--p2p] --copy-storage-all 'qemu+unix:///system?socket=/tmp/migdir/test-sock-driver' 'unix://?socket=/tmp/migdir/test-sock-qemu' [--listen-address /tmp/migdir/test-sock-qemu] --disks-socket /tmp/migdir/test-sock-nbd + </pre>
Why unix://?socket=/tmp/migdir/test-sock-qemu instead of the more obvious unix:///tmp/migdir/test-sock-qemu ? It feels like ?socket is just reinventing the URI path component.
That ?socket syntax makes sense for the libvirtd URIs, because we use the path component to identify the libvirtd instance.
For the migration URI nothing is using the path component, so we should use that for the UNIX path.
I, personally, would prefer to have it unified. Having different way of specifying a unix socket for each parameter is just anti-UX. And since the libvirt URI and NBD URI [1] might use the path component, I did not want to reinvent the wheel of specifying socket paths. And I'm not getting to anonymous sockets and the like. [1] https://github.com/NetworkBlockDevice/nbd/blob/master/doc/uri.md
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Wed, Aug 26, 2020 at 12:14:44AM +0200, Martin Kletzander wrote:
On Tue, Aug 25, 2020 at 04:31:29PM +0100, Daniel P. Berrangé wrote:
On Tue, Aug 25, 2020 at 07:47:14AM +0200, Martin Kletzander wrote:
This allows:
a) migration without access to network
b) complete control of the migration stream
c) easy migration between containerised libvirt daemons on the same host
Resolves: https://bugzilla.redhat.com/1638889
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- docs/manpages/virsh.rst | 17 ++++- docs/migration.html.in | 33 ++++++++++ src/qemu/qemu_migration.c | 128 +++++++++++++++++++++++++++-----------
+virsh migrate web1 [--p2p] --copy-storage-all 'qemu+unix:///system?socket=/tmp/migdir/test-sock-driver' 'unix://?socket=/tmp/migdir/test-sock-qemu' [--listen-address /tmp/migdir/test-sock-qemu] --disks-socket /tmp/migdir/test-sock-nbd + </pre>
Why unix://?socket=/tmp/migdir/test-sock-qemu instead of the more obvious unix:///tmp/migdir/test-sock-qemu ? It feels like ?socket is just reinventing the URI path component.
That ?socket syntax makes sense for the libvirtd URIs, because we use the path component to identify the libvirtd instance.
For the migration URI nothing is using the path component, so we should use that for the UNIX path.
I, personally, would prefer to have it unified. Having different way of specifying a unix socket for each parameter is just anti-UX. And since the libvirt URI and NBD URI [1] might use the path component, I did not want to reinvent the wheel of specifying socket paths. And I'm not getting to anonymous sockets and the like.
Both the libvirt URI and NBD URI are doing multiple different jobs,encoding both the address of the server and a bunch of extra components. Libvirt choose to use the path as a "instance name" which effectively should be thought of as ust an alias/symlink for the socket path. IOW, libvirt should really not have added a "socket" parameter at all, just accepted the UNIX socket path as the URI path. NBD, was dealing with the fact that UNIX path had to be retrofitted after the URI path had alrady seen common use for the export name. If it was not for existing practice, export name would have been better as a URI parameter IMHO In the case of the URIs we need for migration stream and NBD stream, the only thing we need is the address of the target. There's no compelling reason to avoid using the URI path in that sense. I don't find following the libvirt / NBD URI approach compelling because we're not following the rest of their URI scheme, and they are both flawed due to historical mistakes. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Wed, Aug 26, 2020 at 09:20:20AM +0100, Daniel P. Berrangé wrote:
On Wed, Aug 26, 2020 at 12:14:44AM +0200, Martin Kletzander wrote:
On Tue, Aug 25, 2020 at 04:31:29PM +0100, Daniel P. Berrangé wrote:
On Tue, Aug 25, 2020 at 07:47:14AM +0200, Martin Kletzander wrote:
This allows:
a) migration without access to network
b) complete control of the migration stream
c) easy migration between containerised libvirt daemons on the same host
Resolves: https://bugzilla.redhat.com/1638889
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- docs/manpages/virsh.rst | 17 ++++- docs/migration.html.in | 33 ++++++++++ src/qemu/qemu_migration.c | 128 +++++++++++++++++++++++++++-----------
+virsh migrate web1 [--p2p] --copy-storage-all 'qemu+unix:///system?socket=/tmp/migdir/test-sock-driver' 'unix://?socket=/tmp/migdir/test-sock-qemu' [--listen-address /tmp/migdir/test-sock-qemu] --disks-socket /tmp/migdir/test-sock-nbd + </pre>
Why unix://?socket=/tmp/migdir/test-sock-qemu instead of the more obvious unix:///tmp/migdir/test-sock-qemu ? It feels like ?socket is just reinventing the URI path component.
That ?socket syntax makes sense for the libvirtd URIs, because we use the path component to identify the libvirtd instance.
For the migration URI nothing is using the path component, so we should use that for the UNIX path.
I, personally, would prefer to have it unified. Having different way of specifying a unix socket for each parameter is just anti-UX. And since the libvirt URI and NBD URI [1] might use the path component, I did not want to reinvent the wheel of specifying socket paths. And I'm not getting to anonymous sockets and the like.
Both the libvirt URI and NBD URI are doing multiple different jobs,encoding both the address of the server and a bunch of extra components. Libvirt choose to use the path as a "instance name" which effectively should be thought of as ust an alias/symlink for the socket path. IOW, libvirt should really not have added a "socket" parameter at all, just accepted the UNIX socket path as the URI path.
NBD, was dealing with the fact that UNIX path had to be retrofitted after the URI path had alrady seen common use for the export name. If it was not for existing practice, export name would have been better as a URI parameter IMHO
In the case of the URIs we need for migration stream and NBD stream, the only thing we need is the address of the target. There's no compelling reason to avoid using the URI path in that sense. I don't find following the libvirt / NBD URI approach compelling because we're not following the rest of their URI scheme, and they are both flawed due to historical mistakes.
OK, I'll rewrite that.
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Tue, Aug 25, 2020 at 07:47:14 +0200, Martin Kletzander wrote:
This allows:
a) migration without access to network
b) complete control of the migration stream
c) easy migration between containerised libvirt daemons on the same host
Resolves: https://bugzilla.redhat.com/1638889
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- docs/manpages/virsh.rst | 17 ++++- docs/migration.html.in | 33 ++++++++++ src/qemu/qemu_migration.c | 128 +++++++++++++++++++++++++++----------- 3 files changed, 139 insertions(+), 39 deletions(-)
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index cbb3c18deb30..82f7c9f77488 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -3237,12 +3237,12 @@ has different semantics:
In a special circumstance where you require a complete control of the connection and/or libvirt does not have network access to the remote side you can use a -unix transport in the URI and specify a socket path in the query, for example +UNIX transport in the URI and specify a socket path in the query, for example with the qemu driver you could use this:
.. code-block::
- qemu+unix://?socket=/path/to/socket + qemu+unix:///system?socket=/path/to/socket
When *migrateuri* is not specified, libvirt will automatically determine the hypervisor specific URI. Some hypervisors, including QEMU, have an optional
This hunk looks like it should go into the previous patch 7/9 (peer2peer migration: allow connecting to local sockets).
@@ -3270,6 +3270,14 @@ There are a few scenarios where specifying *migrateuri* may help: might be specified to choose a specific port number outside the default range in order to comply with local firewall policies.
+* The *desturi* uses UNIX transport method. In this advanced case libvirt + should not guess a *migrateuri* and it should be specified using + UNIX socket path URI: + +.. code-block:: + + unix://?socket=/path/to/socket + See `https://libvirt.org/migration.html#uris <https://libvirt.org/migration.html#uris>`_ for more details on migration URIs.
@@ -3296,7 +3304,10 @@ specific parameters separated by '&'. Currently recognized parameters are Optional *listen-address* sets the listen address that hypervisor on the destination side should bind to for incoming migration. Both IPv4 and IPv6 addresses are accepted as well as hostnames (the resolving is done on -destination). Some hypervisors do not support this feature and will return an +destination). In niche scenarios you can also use UNIX socket to make the +hypervisor connection over UNIX socket in which case you must make sure the +source can connect to the destination using the socket path provided by you. +Some hypervisors do not support specifying the listen address and will return an error if this parameter is used.
Optional *disks-port* sets the port that hypervisor on destination side should diff --git a/docs/migration.html.in b/docs/migration.html.in index e95ee9de6f1b..9c8417674b22 100644 --- a/docs/migration.html.in +++ b/docs/migration.html.in @@ -201,6 +201,9 @@ numbers. In the latter case the management application may wish to choose a specific port number outside the default range in order to comply with local firewall policies.</li> + <li>The second URI uses UNIX transport method. In this advanced case + libvirt should not guess a *migrateuri* and it should be specified using + UNIX socket path URI: <code>unix://?socket=/path/to/socket</code>.</li> </ol>
<h2><a id="config">Configuration file handling</a></h2> @@ -628,5 +631,35 @@ virsh migrate --p2p --tunnelled web1 qemu+ssh://desthost/system qemu+ssh://10.0. Supported by QEMU driver </p>
+ + <h3><a id="scenariounixsocket">Migration using only UNIX sockets</a></h3> + + <p> + In a niche scenarion where libvirt daemon does not have access to the
s/scenarion/scenario/ or s/a // and s/scenarion/scenarios/
+ network (e.g. running in a restricted container on a host that has + accessible network), when a management application wants to have complete + control over the transfer or when migrating between two containers on the + same host all the communication can be done using UNIX sockets. This + includes connecting to non-standard socket path for the destination + daemon, using UNIX sockets for hypervisor's communication or for the NBD + data transfer. All of that can be used with both peer2peer and direct + migration options. + </p> + + <p> + Example using <code>/tmp/migdir</code> as a directory representing the + same path visible from both libvirt daemons. That can be achieved by + bind-mounting the same directory to different containers running separate + daemons or forwarding connections to these sockets manually + (using <code>socat</code>, <code>netcat</code> or a custom piece of + software): + <pre> +virsh migrate web1 [--p2p] --copy-storage-all 'qemu+unix:///system?socket=/tmp/migdir/test-sock-driver' 'unix://?socket=/tmp/migdir/test-sock-qemu' [--listen-address /tmp/migdir/test-sock-qemu] --disks-socket /tmp/migdir/test-sock-nbd + </pre> + + <p> + Supported by QEMU driver + </p> + </body> </html> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 3f4690f8fb72..1158d152869c 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c ... @@ -3931,16 +3969,34 @@ qemuMigrationSrcPerformNative(virQEMUDriverPtr driver, } }
- /* RDMA and multi-fd migration requires QEMU to connect to the destination - * itself. - */ - if (STREQ(uribits->scheme, "rdma") || (flags & VIR_MIGRATE_PARALLEL)) - spec.destType = MIGRATION_DEST_HOST; - else - spec.destType = MIGRATION_DEST_CONNECT_HOST; - spec.dest.host.protocol = uribits->scheme; - spec.dest.host.name = uribits->server; - spec.dest.host.port = uribits->port; + if (STREQ(uribits->scheme, "unix")) { + if (flags & VIR_MIGRATE_TLS) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("Migration over UNIX socket with TLS is not supported")); + return -1; + } + if (flags & VIR_MIGRATE_PARALLEL) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("Parallel migration over UNIX socket is not supported")); + return -1; + }
From libvirt's POV this should be fairly trivial (can be a follow-up
Is there a reason for not supporting TLS and multi-fd with unix sockets? patch, though): introduce MIGRATION_DEST_SOCKET in addition to MIGRATION_DEST_CONNECT_SOCKET and use it to instruct QEMU to connect using unix: URI (unless this support was removed from QEMU).
+ + spec.destType = MIGRATION_DEST_CONNECT_SOCKET; + spec.dest.socket.path = virURIGetParam(uribits, "socket"); + } else { + /* RDMA and multi-fd migration requires QEMU to connect to the destination + * itself. + */ + if (STREQ(uribits->scheme, "rdma") || (flags & VIR_MIGRATE_PARALLEL)) + spec.destType = MIGRATION_DEST_HOST; + else + spec.destType = MIGRATION_DEST_CONNECT_HOST; + + spec.dest.host.protocol = uribits->scheme; + spec.dest.host.name = uribits->server; + spec.dest.host.port = uribits->port; + } + spec.fwdType = MIGRATION_FWD_DIRECT;
ret = qemuMigrationSrcRun(driver, vm, persist_xml, cookiein, cookieinlen, cookieout,
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
On Tue, Aug 25, 2020 at 05:53:32PM +0200, Jiri Denemark wrote:
On Tue, Aug 25, 2020 at 07:47:14 +0200, Martin Kletzander wrote:
This allows:
a) migration without access to network
b) complete control of the migration stream
c) easy migration between containerised libvirt daemons on the same host
Resolves: https://bugzilla.redhat.com/1638889
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- docs/manpages/virsh.rst | 17 ++++- docs/migration.html.in | 33 ++++++++++ src/qemu/qemu_migration.c | 128 +++++++++++++++++++++++++++----------- 3 files changed, 139 insertions(+), 39 deletions(-)
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index cbb3c18deb30..82f7c9f77488 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -3237,12 +3237,12 @@ has different semantics:
In a special circumstance where you require a complete control of the connection and/or libvirt does not have network access to the remote side you can use a -unix transport in the URI and specify a socket path in the query, for example +UNIX transport in the URI and specify a socket path in the query, for example with the qemu driver you could use this:
.. code-block::
- qemu+unix://?socket=/path/to/socket + qemu+unix:///system?socket=/path/to/socket
When *migrateuri* is not specified, libvirt will automatically determine the hypervisor specific URI. Some hypervisors, including QEMU, have an optional
This hunk looks like it should go into the previous patch 7/9 (peer2peer migration: allow connecting to local sockets).
@@ -3270,6 +3270,14 @@ There are a few scenarios where specifying *migrateuri* may help: might be specified to choose a specific port number outside the default range in order to comply with local firewall policies.
+* The *desturi* uses UNIX transport method. In this advanced case libvirt + should not guess a *migrateuri* and it should be specified using + UNIX socket path URI: + +.. code-block:: + + unix://?socket=/path/to/socket + See `https://libvirt.org/migration.html#uris <https://libvirt.org/migration.html#uris>`_ for more details on migration URIs.
@@ -3296,7 +3304,10 @@ specific parameters separated by '&'. Currently recognized parameters are Optional *listen-address* sets the listen address that hypervisor on the destination side should bind to for incoming migration. Both IPv4 and IPv6 addresses are accepted as well as hostnames (the resolving is done on -destination). Some hypervisors do not support this feature and will return an +destination). In niche scenarios you can also use UNIX socket to make the +hypervisor connection over UNIX socket in which case you must make sure the +source can connect to the destination using the socket path provided by you. +Some hypervisors do not support specifying the listen address and will return an error if this parameter is used.
Optional *disks-port* sets the port that hypervisor on destination side should diff --git a/docs/migration.html.in b/docs/migration.html.in index e95ee9de6f1b..9c8417674b22 100644 --- a/docs/migration.html.in +++ b/docs/migration.html.in @@ -201,6 +201,9 @@ numbers. In the latter case the management application may wish to choose a specific port number outside the default range in order to comply with local firewall policies.</li> + <li>The second URI uses UNIX transport method. In this advanced case + libvirt should not guess a *migrateuri* and it should be specified using + UNIX socket path URI: <code>unix://?socket=/path/to/socket</code>.</li> </ol>
<h2><a id="config">Configuration file handling</a></h2> @@ -628,5 +631,35 @@ virsh migrate --p2p --tunnelled web1 qemu+ssh://desthost/system qemu+ssh://10.0. Supported by QEMU driver </p>
+ + <h3><a id="scenariounixsocket">Migration using only UNIX sockets</a></h3> + + <p> + In a niche scenarion where libvirt daemon does not have access to the
s/scenarion/scenario/
or
s/a // and s/scenarion/scenarios/
+ network (e.g. running in a restricted container on a host that has + accessible network), when a management application wants to have complete + control over the transfer or when migrating between two containers on the + same host all the communication can be done using UNIX sockets. This + includes connecting to non-standard socket path for the destination + daemon, using UNIX sockets for hypervisor's communication or for the NBD + data transfer. All of that can be used with both peer2peer and direct + migration options. + </p> + + <p> + Example using <code>/tmp/migdir</code> as a directory representing the + same path visible from both libvirt daemons. That can be achieved by + bind-mounting the same directory to different containers running separate + daemons or forwarding connections to these sockets manually + (using <code>socat</code>, <code>netcat</code> or a custom piece of + software): + <pre> +virsh migrate web1 [--p2p] --copy-storage-all 'qemu+unix:///system?socket=/tmp/migdir/test-sock-driver' 'unix://?socket=/tmp/migdir/test-sock-qemu' [--listen-address /tmp/migdir/test-sock-qemu] --disks-socket /tmp/migdir/test-sock-nbd + </pre> + + <p> + Supported by QEMU driver + </p> + </body> </html> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 3f4690f8fb72..1158d152869c 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c ... @@ -3931,16 +3969,34 @@ qemuMigrationSrcPerformNative(virQEMUDriverPtr driver, } }
- /* RDMA and multi-fd migration requires QEMU to connect to the destination - * itself. - */ - if (STREQ(uribits->scheme, "rdma") || (flags & VIR_MIGRATE_PARALLEL)) - spec.destType = MIGRATION_DEST_HOST; - else - spec.destType = MIGRATION_DEST_CONNECT_HOST; - spec.dest.host.protocol = uribits->scheme; - spec.dest.host.name = uribits->server; - spec.dest.host.port = uribits->port; + if (STREQ(uribits->scheme, "unix")) { + if (flags & VIR_MIGRATE_TLS) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("Migration over UNIX socket with TLS is not supported")); + return -1; + } + if (flags & VIR_MIGRATE_PARALLEL) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("Parallel migration over UNIX socket is not supported")); + return -1; + }
From libvirt's POV this should be fairly trivial (can be a follow-up
Is there a reason for not supporting TLS and multi-fd with unix sockets? patch, though): introduce MIGRATION_DEST_SOCKET in addition to MIGRATION_DEST_CONNECT_SOCKET and use it to instruct QEMU to connect using unix: URI (unless this support was removed from QEMU).
multi-fd is certainly desirable to support and I don't see any reason to block that. TLS is more problematic, at least if you are using x509 credentials then you need to tell QEMU what hostname to use for validation. This would require us to accept a hostname parameter in the URI giving the UNIX socket. This reminds me, however, that we should add support for PSK credentials in libvirt. PSK is an alternative to x509 where you just provide QEMU the private keypair directly. To use PSK you need to have a trusted channel to both src and dst over which libvirt can send the key. ...fortunately we should have that already as libvirtd RPC always uses a secure connection between libvirtds for sending migration cookies, etc. Libvirtd just needs to generate the key which is trivial. If we add support for PSK, then a user can use TLS encrypted migration out of the box with zero config required, if they're using SSH tunnel for libvirtd.
+ + spec.destType = MIGRATION_DEST_CONNECT_SOCKET; + spec.dest.socket.path = virURIGetParam(uribits, "socket"); + } else { + /* RDMA and multi-fd migration requires QEMU to connect to the destination + * itself. + */ + if (STREQ(uribits->scheme, "rdma") || (flags & VIR_MIGRATE_PARALLEL)) + spec.destType = MIGRATION_DEST_HOST; + else + spec.destType = MIGRATION_DEST_CONNECT_HOST; + + spec.dest.host.protocol = uribits->scheme; + spec.dest.host.name = uribits->server; + spec.dest.host.port = uribits->port; + } + spec.fwdType = MIGRATION_FWD_DIRECT;
ret = qemuMigrationSrcRun(driver, vm, persist_xml, cookiein, cookieinlen, cookieout,
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Tue, Aug 25, 2020 at 17:12:59 +0100, Daniel P. Berrangé wrote:
On Tue, Aug 25, 2020 at 05:53:32PM +0200, Jiri Denemark wrote:
On Tue, Aug 25, 2020 at 07:47:14 +0200, Martin Kletzander wrote:
- /* RDMA and multi-fd migration requires QEMU to connect to the destination - * itself. - */ - if (STREQ(uribits->scheme, "rdma") || (flags & VIR_MIGRATE_PARALLEL)) - spec.destType = MIGRATION_DEST_HOST; - else - spec.destType = MIGRATION_DEST_CONNECT_HOST; - spec.dest.host.protocol = uribits->scheme; - spec.dest.host.name = uribits->server; - spec.dest.host.port = uribits->port; + if (STREQ(uribits->scheme, "unix")) { + if (flags & VIR_MIGRATE_TLS) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("Migration over UNIX socket with TLS is not supported")); + return -1; + } + if (flags & VIR_MIGRATE_PARALLEL) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("Parallel migration over UNIX socket is not supported")); + return -1; + }
From libvirt's POV this should be fairly trivial (can be a follow-up
Is there a reason for not supporting TLS and multi-fd with unix sockets? patch, though): introduce MIGRATION_DEST_SOCKET in addition to MIGRATION_DEST_CONNECT_SOCKET and use it to instruct QEMU to connect using unix: URI (unless this support was removed from QEMU).
multi-fd is certainly desirable to support and I don't see any reason to block that.
TLS is more problematic, at least if you are using x509 credentials then you need to tell QEMU what hostname to use for validation. This would require us to accept a hostname parameter in the URI giving the UNIX socket.
We already support this generally regardless on the transport via VIR_MIGRATE_PARAM_TLS_DESTINATION parameter. Jirka
On Tue, Aug 25, 2020 at 06:34:05PM +0200, Jiri Denemark wrote:
On Tue, Aug 25, 2020 at 17:12:59 +0100, Daniel P. Berrangé wrote:
On Tue, Aug 25, 2020 at 05:53:32PM +0200, Jiri Denemark wrote:
On Tue, Aug 25, 2020 at 07:47:14 +0200, Martin Kletzander wrote:
- /* RDMA and multi-fd migration requires QEMU to connect to the destination - * itself. - */ - if (STREQ(uribits->scheme, "rdma") || (flags & VIR_MIGRATE_PARALLEL)) - spec.destType = MIGRATION_DEST_HOST; - else - spec.destType = MIGRATION_DEST_CONNECT_HOST; - spec.dest.host.protocol = uribits->scheme; - spec.dest.host.name = uribits->server; - spec.dest.host.port = uribits->port; + if (STREQ(uribits->scheme, "unix")) { + if (flags & VIR_MIGRATE_TLS) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("Migration over UNIX socket with TLS is not supported")); + return -1; + } + if (flags & VIR_MIGRATE_PARALLEL) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("Parallel migration over UNIX socket is not supported")); + return -1; + }
From libvirt's POV this should be fairly trivial (can be a follow-up
Is there a reason for not supporting TLS and multi-fd with unix sockets? patch, though): introduce MIGRATION_DEST_SOCKET in addition to MIGRATION_DEST_CONNECT_SOCKET and use it to instruct QEMU to connect using unix: URI (unless this support was removed from QEMU).
multi-fd is certainly desirable to support and I don't see any reason to block that.
TLS is more problematic, at least if you are using x509 credentials then you need to tell QEMU what hostname to use for validation. This would require us to accept a hostname parameter in the URI giving the UNIX socket.
We already support this generally regardless on the transport via VIR_MIGRATE_PARAM_TLS_DESTINATION parameter.
Ah yes, I forgot. In that case, I don't see a obvious reason to block TLS. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Tue, Aug 25, 2020 at 05:35:52PM +0100, Daniel P. Berrangé wrote:
On Tue, Aug 25, 2020 at 06:34:05PM +0200, Jiri Denemark wrote:
On Tue, Aug 25, 2020 at 17:12:59 +0100, Daniel P. Berrangé wrote:
On Tue, Aug 25, 2020 at 05:53:32PM +0200, Jiri Denemark wrote:
On Tue, Aug 25, 2020 at 07:47:14 +0200, Martin Kletzander wrote:
- /* RDMA and multi-fd migration requires QEMU to connect to the destination - * itself. - */ - if (STREQ(uribits->scheme, "rdma") || (flags & VIR_MIGRATE_PARALLEL)) - spec.destType = MIGRATION_DEST_HOST; - else - spec.destType = MIGRATION_DEST_CONNECT_HOST; - spec.dest.host.protocol = uribits->scheme; - spec.dest.host.name = uribits->server; - spec.dest.host.port = uribits->port; + if (STREQ(uribits->scheme, "unix")) { + if (flags & VIR_MIGRATE_TLS) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("Migration over UNIX socket with TLS is not supported")); + return -1; + } + if (flags & VIR_MIGRATE_PARALLEL) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("Parallel migration over UNIX socket is not supported")); + return -1; + }
From libvirt's POV this should be fairly trivial (can be a follow-up
Is there a reason for not supporting TLS and multi-fd with unix sockets? patch, though): introduce MIGRATION_DEST_SOCKET in addition to MIGRATION_DEST_CONNECT_SOCKET and use it to instruct QEMU to connect using unix: URI (unless this support was removed from QEMU).
multi-fd is certainly desirable to support and I don't see any reason to block that.
TLS is more problematic, at least if you are using x509 credentials then you need to tell QEMU what hostname to use for validation. This would require us to accept a hostname parameter in the URI giving the UNIX socket.
We already support this generally regardless on the transport via VIR_MIGRATE_PARAM_TLS_DESTINATION parameter.
Ah yes, I forgot. In that case, I don't see a obvious reason to block TLS.
I have got two reasons, both of which are kinda stupid, so I will remove the check. The reasons are: 1) Due to my limited knowledge I was not sure whether "just allowing it" is enough or whether I need to do something extra (like requiring VIR_MIGRATE_PARAM_TLS_DESTINATION) and testing this already took me enough time and 2) since the mgmt app (or user, if there's ever going to be anyone crazy enough) can (and actually needs to) manage the data path it can add its own encryption, compression etc. as they are in the full control of what happens with the data and they will probably not bother adding anything extra on top of making sure the sockets refer to the same file or simply proxying the data stream (I'm quite sure the mgmt app asking for this won't). I am fine with adding support for something nobody might use as we cannot ever be sure. This, however, changes when we are trying to be completely backward compatible as it could mean we need to support it forever. But if checking the one typed param is enough for TLS and multi-fd should "just work" I'm fine with rewriting the check.
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- NEWS.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index d92714c29b88..49c476be5d51 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -20,6 +20,12 @@ v6.7.0 (unreleased) Sparse streams (e.g. ``virsh vol-download --sparse`` or ``virsh vol-upload --sparse``) now handle if one of the stream ends is a block device. + * qemu: Allow migration over UNIX sockets + + QEMU migration can now be performed completely over UNIX sockets. This is + useful for containerised scenarios and can be used in both peer2peer and + direct migrations. + * **Bug fixes** * virdevmapper: Deal with kernels without DM support -- 2.28.0
On Tue, Aug 25, 2020 at 07:47:15 +0200, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- NEWS.rst | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/NEWS.rst b/NEWS.rst index d92714c29b88..49c476be5d51 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -20,6 +20,12 @@ v6.7.0 (unreleased) Sparse streams (e.g. ``virsh vol-download --sparse`` or ``virsh vol-upload --sparse``) now handle if one of the stream ends is a block device.
+ * qemu: Allow migration over UNIX sockets + + QEMU migration can now be performed completely over UNIX sockets. This is + useful for containerised scenarios and can be used in both peer2peer and + direct migrations. + * **Bug fixes**
* virdevmapper: Deal with kernels without DM support
Reviewed-by: Jiri Denemark <jdenemar@redhat.com> But see additional notes from Daniel (and me) which invalidate my Reviewed-by tags for patches 6 and 8.
participants (3)
-
Daniel P. Berrangé -
Jiri Denemark -
Martin Kletzander