10:03 a.m.
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(a)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(a)redhat.com>