[libvirt] [RFC PATCH v2 2/3] qemu: RDMA migration support using 'rdma' URI
Jiri Denemark
jdenemar at redhat.com
Mon Feb 3 15:19:24 UTC 2014
On Mon, Jan 13, 2014 at 14:28:11 +0800, mrhines at linux.vnet.ibm.com wrote:
> From: "Michael R. Hines" <mrhines at us.ibm.com>
>
> The switch from x-rdma => rdma has not yet happened,
> but at least we can review the patch until it goes
> through on qemu-devel.
The paragraph above would better fit after "---" below so that it
disappears once this patch gets applied as the statement won't be valid
anymore at that time.
> 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
> Full documenation of the feature: http://wiki.qemu.org/Features/RDMALiveMigration
>
> This patch includes mainly making all the locations in
> libvirt where the 'tcp' string was hard-coded to be
> more flexible to use more than one protocol.
>
> Signed-off-by: Michael R. Hines <mrhines at us.ibm.com>
> ---
> 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 (!(qemuCaps = virQEMUCapsCacheLookupCopy(driver->qemuCapsCache,
> (*def)->emulator)))
> goto cleanup;
> @@ -2318,9 +2330,9 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
> * -incoming <IPv4 addr>:port or -incoming <hostname>:port
> */
> if ((encloseAddress &&
> - virAsprintf(&migrateFrom, "tcp:[%s]:%d", listenAddress, port) < 0) ||
> + virAsprintf(&migrateFrom, "%s:[%s]:%d", protocol, listenAddress, port) < 0) ||
> (!encloseAddress &&
> - virAsprintf(&migrateFrom, "tcp:%s:%d", listenAddress, port) < 0))
> + virAsprintf(&migrateFrom, "%s:%s:%d", protocol, listenAddress, port) < 0))
> goto cleanup;
> }
>
> @@ -2507,7 +2519,7 @@ qemuMigrationPrepareTunnel(virQEMUDriverPtr driver,
>
> ret = qemuMigrationPrepareAny(driver, dconn, cookiein, cookieinlen,
> cookieout, cookieoutlen, def, origname,
> - st, 0, false, NULL, flags);
> + st, "tcp", 0, false, NULL, flags);
> return ret;
> }
>
> @@ -2529,6 +2541,8 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver,
> unsigned short port = 0;
> bool autoPort = true;
> char *hostname = NULL;
> + const char *protocol = NULL;
> + char *well_formed_protocol = NULL;
> const char *p;
> char *uri_str = NULL;
> int ret = -1;
> @@ -2577,21 +2591,37 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver,
> if (virAsprintf(uri_out, "tcp:%s:%d", hostname, port) < 0)
> goto cleanup;
> } else {
> - /* Check the URI starts with "tcp:". We will escape the
> + char * protocol_save = NULL;
> + char * uri_save = NULL;
> +
> + /* Check the URI starts with a valid prefix. We will escape the
> * URI when passing it to the qemu monitor, so bad
> * characters in hostname part don't matter.
> */
> - 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.
>
> @@ -2637,10 +2667,13 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver,
>
> ret = qemuMigrationPrepareAny(driver, dconn, cookiein, cookieinlen,
> cookieout, cookieoutlen, def, origname,
> - NULL, port, autoPort, listenAddress, flags);
> + NULL, protocol ? protocol : "tcp",
> + port, autoPort, listenAddress, flags);
> cleanup:
> virURIFree(uri);
> VIR_FREE(hostname);
> + VIR_FREE(protocol);
> + VIR_FREE(well_formed_protocol);
> if (ret != 0) {
> VIR_FREE(*uri_out);
> if (autoPort)
> @@ -2844,6 +2877,7 @@ struct _qemuMigrationSpec {
> enum qemuMigrationDestinationType destType;
> union {
> struct {
> + const char *proto;
> const char *name;
> int port;
> } host;
> @@ -3205,6 +3239,7 @@ qemuMigrationRun(virQEMUDriverPtr driver,
> switch (spec->destType) {
> case MIGRATION_DEST_HOST:
> ret = qemuMonitorMigrateToHost(priv->mon, migrate_flags,
> + spec->dest.host.proto,
> spec->dest.host.name,
> spec->dest.host.port);
> break;
> @@ -3335,7 +3370,7 @@ cancel:
> goto cleanup;
> }
>
> -/* Perform migration using QEMU's native TCP migrate support,
> +/* Perform migration using QEMU's native migrate support,
> * not encrypted obviously
> */
> static int doNativeMigrate(virQEMUDriverPtr driver,
> @@ -3353,7 +3388,11 @@ static int doNativeMigrate(virQEMUDriverPtr driver,
> qemuDomainObjPrivatePtr priv = vm->privateData;
> virURIPtr uribits = NULL;
> int ret = -1;
> + char *tmp = NULL;
> qemuMigrationSpec spec;
> + char *well_formed_proto = NULL;
> + char * protocol_save = NULL;
> + char * uri_save = NULL;
>
> VIR_DEBUG("driver=%p, vm=%p, uri=%s, cookiein=%s, cookieinlen=%d, "
> "cookieout=%p, cookieoutlen=%p, flags=%lx, resource=%lu, "
> @@ -3362,20 +3401,44 @@ static int doNativeMigrate(virQEMUDriverPtr driver,
> cookieout, cookieoutlen, flags, resource,
> NULLSTR(graphicsuri));
>
> - if (STRPREFIX(uri, "tcp:") && !STRPREFIX(uri, "tcp://")) {
> - char *tmp;
> - /* HACK: source host generates bogus URIs, so fix them up */
> - if (virAsprintf(&tmp, "tcp://%s", uri + strlen("tcp:")) < 0)
> + ret = VIR_STRDUP(uri_save, uri);
> + if (ret <= 0) {
> + return -1;
> + }
> +
> + spec.dest.host.proto = strtok_r(uri_save, ":", &protocol_save);
> + VIR_FREE(uri_save);
> +
> + /* HACK: source host generates bogus URIs, so fix them up */
> + if (spec.dest.host.proto) {
> + ret = virAsprintf(&well_formed_proto, "%s://",
> + spec.dest.host.proto);
> + if (ret < 0)
> + return ret;
> + }
> +
> + /* 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);
This change is definitely not OK. virURIParse is not a place such hacks.
Jirka
More information about the libvir-list
mailing list