Re: [libvirt] [RFC PATCH v2 2/3] qemu: RDMA migration support using 'rdma' URI

> USAGE: $ virsh migrate --live --migrateuri x-rdma:hostname domain qemu+ssh://hostname/system s/x-rdma/rdma/ and I believe we should use rdma://hostname as the URI

> Signed-off-by: Michael R. Hines <mrhines(a)us.ibm.com&gt; > --- > src/qemu/qemu_capabilities.c | 13 +++++ > src/qemu/qemu_capabilities.h | 1 + > src/qemu/qemu_command.c | 8 ++++ > src/qemu/qemu_migration.c | 110 ++++++++++++++++++++++++++++++++++--------- > src/qemu/qemu_monitor.c | 3 +- > src/qemu/qemu_monitor.h | 1 + > src/util/viruri.c | 7 ++- > 7 files changed, 119 insertions(+), 24 deletions(-) > > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c > index 548b988..d82b48c 100644 > --- a/src/qemu/qemu_capabilities.c > +++ b/src/qemu/qemu_capabilities.c > @@ -243,6 +243,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, > "virtio-mmio", > "ich9-intel-hda", > "kvm-pit-lost-tick-policy", > + > + "migrate-qemu-rdma", /* 160 */ s/migrate-qemu-rdma/migrate-rdma/ "qemu" string in pretty redundant here given that it is a qemu capability.

> ); > > struct _virQEMUCaps { > @@ -906,6 +908,9 @@ virCapsPtr virQEMUCapsInit(virQEMUCapsCachePtr cache) > virCapabilitiesAddHostMigrateTransport(caps, > "tcp"); > > + virCapabilitiesAddHostMigrateTransport(caps, > + "rdma"); > + > /* QEMU can support pretty much every arch that exists, > * so just probe for them all - we gracefully fail > * if a qemu-system-$ARCH binary can't be found > @@ -1110,6 +1115,7 @@ virQEMUCapsComputeCmdFlags(const char *help, > * -incoming unix (qemu >= 0.12.0) > * -incoming fd (qemu >= 0.12.0) > * -incoming stdio (all earlier kvm) > + * -incoming rdma (qemu >= 2.0.0) > * > * NB, there was a pre-kvm-79 'tcp' support, but it > * was broken, because it blocked the monitor console > @@ -1130,6 +1136,9 @@ virQEMUCapsComputeCmdFlags(const char *help, > virQEMUCapsSet(qemuCaps, QEMU_CAPS_MIGRATE_KVM_STDIO); > } > > + if (version >= 2000000) > + virQEMUCapsSet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_RDMA); > + > if (version >= 10000) > virQEMUCapsSet(qemuCaps, QEMU_CAPS_0_10); > This is not needed, we won't be parsing -help for any QEMU that supports RDMA.

> @@ -2561,6 +2570,10 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, > if (qemuCaps->version >= 1006000) > virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY); > > + if (qemuCaps->version >= 2000000) > + virQEMUCapsSet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_RDMA); > + > + And here we need a better check for rdma migration. What if someone compiles QEMU without RDMA support? > if (virQEMUCapsProbeQMPCommands(qemuCaps, mon) < 0) > goto cleanup; > if (virQEMUCapsProbeQMPEvents(qemuCaps, mon) < 0) > diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h > index 02d47c6..3e78961 100644 > --- a/src/qemu/qemu_capabilities.h > +++ b/src/qemu/qemu_capabilities.h > @@ -198,6 +198,7 @@ enum virQEMUCapsFlags { > QEMU_CAPS_DEVICE_VIRTIO_MMIO = 157, /* -device virtio-mmio */ > QEMU_CAPS_DEVICE_ICH9_INTEL_HDA = 158, /* -device ich9-intel-hda */ > QEMU_CAPS_KVM_PIT_TICK_POLICY = 159, /* kvm-pit.lost_tick_policy */ > + QEMU_CAPS_MIGRATE_QEMU_RDMA = 160, /* have qemu rdma migration */ s/_QEMU// as it is redundant.

> > QEMU_CAPS_LAST, /* this must always be the last item */ > }; > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index 763417f..0d23d8b 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -9448,6 +9448,14 @@ qemuBuildCommandLine(virConnectPtr conn, > goto error; > } > virCommandAddArg(cmd, migrateFrom); > + } else if (STRPREFIX(migrateFrom, "rdma")) { > + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_RDMA)) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + "%s", _("RDMA migration is not supported with " > + "this QEMU binary")); > + goto error; > + } > + virCommandAddArg(cmd, migrateFrom); > } else if (STREQ(migrateFrom, "stdio")) { > if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_FD)) { > virCommandAddArgFormat(cmd, "fd:%d", migrateFd); > diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c > index ef6f1c5..1e0f538 100644 > --- a/src/qemu/qemu_migration.c > +++ b/src/qemu/qemu_migration.c > @@ -46,6 +46,7 @@ > #include "virerror.h" > #include "viralloc.h" > #include "virfile.h" > +#include "virprocess.h" > #include "datatypes.h" > #include "fdstream.h" > #include "viruuid.h" > @@ -2163,6 +2164,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, > virDomainDefPtr *def, > const char *origname, > virStreamPtr st, > + const char *protocol, > unsigned short port, > bool autoPort, > const char *listenAddress, > @@ -2275,6 +2277,16 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, > freeaddrinfo(info); > hostIPv6Capable = true; > } > + > + /* > + * RDMA (iWarp) until linux 3.11 is broken, need > + * better host librdmacm IPv6 support detection. > + * Disallow by default for now if RDMA. > + */ > + if (hostIPv6Capable && strstr(protocol, "rdma")) { > + hostIPv6Capable = false; > + } > + Is this still needed? 3.11 will already be quite old when QEMU with RDMA support is released...

> - if (!(p = STRSKIP(uri_in, "tcp:"))) { > - virReportError(VIR_ERR_INVALID_ARG, "%s", > - _("only tcp URIs are supported for KVM/QEMU" > - " migrations")); > + > + if (VIR_STRDUP(uri_save, uri_in) <= 0) { > goto cleanup; > } > > - /* Convert uri_in to well-formed URI with // after tcp: */ > - if (!(STRPREFIX(uri_in, "tcp://"))) { > + protocol = strtok_r(uri_save, ":", &protocol_save); > + VIR_FREE(uri_save); > + if (protocol) { > + if (virAsprintf(&well_formed_protocol, "%s://", protocol) < 0) > + goto cleanup; > + } > + > + /* Make sure it's a valid protocol */ > + if (!(p = STRSKIP(uri_in, "tcp:")) && > + !(p = STRSKIP(uri_in, "rdma:"))) { > + virReportError(VIR_ERR_INVALID_ARG, _("URI %s (%s) not supported" > + " for KVM/QEMU migrations"), protocol, uri_in); > + goto cleanup; > + } > + > + /* Convert uri_in to well-formed URI with // after colon */ > + if (!(STRPREFIX(uri_in, well_formed_protocol))) { > well_formed_uri = false; > - if (virAsprintf(&uri_str, "tcp://%s", p) < 0) > + if (virAsprintf(&uri_str, "%s://%s", protocol, p) < 0) > goto cleanup; > } As I already said several month ago, "tcp:hostname" was a mistake and we should not be redoing it with rdma. Thus I think we should just check for "tcp:" and turn it into "tcp://" and then parse the result as a proper URI. RDMA should always use rdma:// to avoid this backward compatibility hacks.

+ + /* HACK: source host generates bogus URIs, so fix them up */ + + if (!STRPREFIX(uri, well_formed_proto)) { + if (virAsprintf(&tmp, "%s://%s", spec.dest.host.proto, + uri + strlen(spec.dest.host.proto) + 1) < 0) return -1; - uribits = virURIParse(tmp); - VIR_FREE(tmp); } else { uribits = virURIParse(uri); } And the same applies here. We should keep the hack only for "tcp:" URIs and pass all other URIs just straight to virURIParse.

> - if (!uribits) > - return -1; > > - if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATE_QEMU_FD)) > + if (tmp) { > + uribits = virURIParse(tmp); > + VIR_FREE(tmp); > + } > + > + if (!uribits) { > + ret = -1; > + goto err; > + } > + > + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATE_QEMU_FD) && > + !STREQ(spec.dest.host.proto, "rdma")) Indentation is 4 spaces off. > spec.destType = MIGRATION_DEST_CONNECT_HOST; > else > spec.destType = MIGRATION_DEST_HOST; > @@ -3392,6 +3455,9 @@ static int doNativeMigrate(virQEMUDriverPtr driver, > > virURIFree(uribits); > > +err: > + VIR_FREE(well_formed_proto); > + > return ret; > } > > diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c > index 1514715..5a450e2 100644 > --- a/src/qemu/qemu_monitor.c > +++ b/src/qemu/qemu_monitor.c > @@ -2164,6 +2164,7 @@ int qemuMonitorMigrateToFd(qemuMonitorPtr mon, > > int qemuMonitorMigrateToHost(qemuMonitorPtr mon, > unsigned int flags, > + const char *proto, > const char *hostname, > int port) > { > @@ -2179,7 +2180,7 @@ int qemuMonitorMigrateToHost(qemuMonitorPtr mon, > } > > > - if (virAsprintf(&uri, "tcp:%s:%d", hostname, port) < 0) > + if (virAsprintf(&uri, "%s:%s:%d", proto, hostname, port) < 0) > return -1; > > if (mon->json) > diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h > index 27f9cb4..16b0b77 100644 > --- a/src/qemu/qemu_monitor.h > +++ b/src/qemu/qemu_monitor.h > @@ -476,6 +476,7 @@ int qemuMonitorMigrateToFd(qemuMonitorPtr mon, > > int qemuMonitorMigrateToHost(qemuMonitorPtr mon, > unsigned int flags, > + const char *proto, > const char *hostname, > int port); > > diff --git a/src/util/viruri.c b/src/util/viruri.c > index 35efad8..662029a 100644 > --- a/src/util/viruri.c > +++ b/src/util/viruri.c > @@ -187,7 +187,12 @@ virURIParse(const char *uri) > goto error; > > /* First check: does it even make sense to jump inside */ > - if (ret->server != NULL && > + > + /* > + * IPv6 rdma over iwarp is broken in linux. Waiting for a > + * fix on the kernel side... > + */ > + if (ret->server != NULL && !STREQ(ret->scheme, "rdma") && > ret->server[0] == '[') { > size_t length = strlen(ret->server);