On Fri, Jul 26, 2013 at 13:47:43 -0400, mrhines(a)linux.vnet.ibm.com wrote: NACK You can't change content of existing C structures that are part or public API. Running make syntax-check will warn you when you try to do that. Look at virDomainGetJobStats (commits v1.0.2-239-g4dd00f4 to v1.0.2-244-g4121a77) which can return extended migration statistics. Jirka

On 07/26/2013 11:47 AM, mrhines(a)linux.vnet.ibm.com wrote: This series adds a new feature, and we're already in freeze for 1.1.1, so it would have to be post-freeze. NACK; this is a change to ABI. We cannot change the existing struct size without invalidating apps compiled against an earlier version of the library (ie. virDomainGetJobInfo is cast in stone). Rather, we should be adding new VIR_DOMAIN_JOB_* macros for use in the virTypedParameters of virDomainGetJobStats(). It does point out a bug that can be fixed during freeze: we should fix our documentation so that virDomainGetJobInfo() refers to the better virDomainGetJobStats(). Somewhat related: I mentioned in another thread that neither virDomainGetJob{Info,Stats} nor virDomainGetBlockJobInfo play nicely with the idea of being able to have multiple jobs running in parallel, which is necessary if we want to be able to support fleecing of point-in-time snapshots from multiple clients. Ultimately, that means I think we need to enhance the job mechanism so that new jobs return a handle, then we add new API that can query and abort jobs based on the handle instead of being limited to only the current job; I guess for back-compat, it would also help to have an API that sets the current job out of a set of jobs so that the old APIs are still usable. If we do an upgrade of the job API, we would make sure that the new query method sticks to virTypedParameters in the same was as virDomainGetJobStats(). -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

From: "Michael R. Hines" <mrhines(a)us.ibm.com&gt; QEMU has in tree now for version 1.6 support for RDMA Live migration. 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. While the RDMA protocol has been extensively tested (from multiple companies as well as virt-test), the protocol 'x-rdma' will later be renamed to 'rdma' after the community has allowed the feature more cooking. Example usage: virsh migrate --live --migrateuri x-rdma:hostname domain qemu+tcp://hostname/system Signed-off-by: Michael R. Hines <mrhines(a)us.ibm.com&gt; --- src/qemu/qemu_capabilities.c | 7 ++++ src/qemu/qemu_capabilities.h | 4 +++ src/qemu/qemu_command.c | 8 +++++ src/qemu/qemu_migration.c | 75 +++++++++++++++++++++++++++++++----------- src/qemu/qemu_monitor.c | 3 +- src/qemu/qemu_monitor.h | 1 + 6 files changed, 78 insertions(+), 20 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 5dc3c9e..94d17c6 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -234,6 +234,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "vnc-share-policy", /* 150 */ "device-del-event", + + "x-rdma", /* 152 */ ); struct _virQEMUCaps { @@ -1101,6 +1103,7 @@ virQEMUCapsComputeCmdFlags(const char *help, * -incoming unix (qemu >= 0.12.0) * -incoming fd (qemu >= 0.12.0) * -incoming stdio (all earlier kvm) + * -incoming x-rdma (qemu >= 1.6.0) * * NB, there was a pre-kvm-79 'tcp' support, but it * was broken, because it blocked the monitor console @@ -2437,6 +2440,10 @@ virQEMUCapsInitArchQMPBasic(virQEMUCapsPtr qemuCaps, char *archstr = NULL; int ret = -1; + if (qemuCaps->version >= MIN_X_RDMA_VERSION) { + virQEMUCapsSet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_X_RDMA); + } + if (!(archstr = qemuMonitorGetTargetArch(mon))) return -1; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index f5f685d..5069552 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -191,9 +191,13 @@ enum virQEMUCapsFlags { QEMU_CAPS_VNC_SHARE_POLICY = 150, /* set display sharing policy */ QEMU_CAPS_DEVICE_DEL_EVENT = 151, /* DEVICE_DELETED event */ + QEMU_CAPS_MIGRATE_QEMU_X_RDMA = 152, /* have qemu x-rdma migration */ + QEMU_CAPS_LAST, /* this must always be the last item */ }; +#define MIN_X_RDMA_VERSION 1006000 + typedef struct _virQEMUCaps virQEMUCaps; typedef virQEMUCaps *virQEMUCapsPtr; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index aa3a2fd..a26acd7 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8657,6 +8657,14 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } virCommandAddArg(cmd, migrateFrom); + } else if (STRPREFIX(migrateFrom, "x-rdma")) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_X_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 19001b9..de20d23 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2169,7 +2169,8 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, virDomainDefPtr *def, virStreamPtr st, unsigned int port, - unsigned long flags) + unsigned long flags, + const char *protocol) { virDomainObjPtr vm = NULL; virDomainEventPtr event = NULL; @@ -2280,7 +2281,8 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, * and there is at least one IPv6 address configured */ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_IPV6_MIGRATION) && - getaddrinfo("::", NULL, &hints, &info) == 0) { + getaddrinfo("::", NULL, &hints, &info) == 0 && + !strstr(protocol, "rdma")) { freeaddrinfo(info); listenAddr = "[::]"; } else { @@ -2291,7 +2293,8 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, /* QEMU will be started with -incoming [::]:port * or -incoming 0.0.0.0:port */ - if (virAsprintf(&migrateFrom, "tcp:%s:%d", listenAddr, port) < 0) + if (virAsprintf(&migrateFrom, "%s:%s:%d", protocol, + listenAddr, port) < 0) goto cleanup; } @@ -2482,7 +2485,7 @@ qemuMigrationPrepareTunnel(virQEMUDriverPtr driver, ret = qemuMigrationPrepareAny(driver, dconn, cookiein, cookieinlen, cookieout, cookieoutlen, def, - st, 0, flags); + st, 0, flags, "tcp"); return ret; } @@ -2502,6 +2505,8 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver, static int port = 0; int this_port; char *hostname = NULL; + const char *protocol = NULL; + char *well_formed_protocol = NULL; const char *p; char *uri_str = NULL; int ret = -1; @@ -2550,20 +2555,29 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver, if (virAsprintf(uri_out, "tcp:%s:%d", hostname, this_port) < 0) goto cleanup; } else { - /* Check the URI starts with "tcp:". We will escape the + /* 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")); + + protocol = strtok(strdup(uri_in), ":"); + 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, "x-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 tcp: */ - if (!(STRPREFIX(uri_in, "tcp://"))) { - if (virAsprintf(&uri_str, "tcp://%s", p) < 0) + + /* Convert uri_in to well-formed URI with // after colon */ + if (!(STRPREFIX(uri_in, well_formed_protocol))) { + if (virAsprintf(&uri_str, "%s://%s", protocol, p) < 0) goto cleanup; } @@ -2602,10 +2616,20 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver, ret = qemuMigrationPrepareAny(driver, dconn, cookiein, cookieinlen, cookieout, cookieoutlen, def, - NULL, this_port, flags); + NULL, this_port, flags, + protocol ? protocol : "tcp"); cleanup: virURIFree(uri); VIR_FREE(hostname); + + if (protocol) { + VIR_FREE(protocol); + } + + if (well_formed_protocol) { + VIR_FREE(well_formed_protocol); + } + if (ret != 0) VIR_FREE(*uri_out); return ret; @@ -2800,6 +2824,7 @@ struct _qemuMigrationSpec { enum qemuMigrationDestinationType destType; union { struct { + const char *proto; const char *name; int port; } host; @@ -3161,6 +3186,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; @@ -3291,7 +3317,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, @@ -3309,6 +3335,8 @@ static int doNativeMigrate(virQEMUDriverPtr driver, qemuDomainObjPrivatePtr priv = vm->privateData; virURIPtr uribits = NULL; int ret = -1; + char *tmp = NULL; + bool rdma = false; qemuMigrationSpec spec; VIR_DEBUG("driver=%p, vm=%p, uri=%s, cookiein=%s, cookieinlen=%d, " @@ -3318,20 +3346,29 @@ static int doNativeMigrate(virQEMUDriverPtr driver, cookieout, cookieoutlen, flags, resource, NULLSTR(graphicsuri)); + /* HACK: source host generates bogus URIs, so fix them up */ 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) return -1; - uribits = virURIParse(tmp); - VIR_FREE(tmp); + spec.dest.host.proto = "tcp"; + } else if (STRPREFIX(uri, "x-rdma:") && !STRPREFIX(uri, "x-rdma://")) { + if (virAsprintf(&tmp, "x-rdma://%s", uri + strlen("x-rdma:")) < 0) + return -1; + rdma = true; + spec.dest.host.proto = "x-rdma"; } else { uribits = virURIParse(uri); } + + if (tmp) { + uribits = virURIParse(tmp); + VIR_FREE(tmp); + } + if (!uribits) return -1; - if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATE_QEMU_FD)) + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATE_QEMU_FD) && !rdma) spec.destType = MIGRATION_DEST_CONNECT_HOST; else spec.destType = MIGRATION_DEST_HOST; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 86aed75..ce95174 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2098,6 +2098,7 @@ int qemuMonitorMigrateToFd(qemuMonitorPtr mon, int qemuMonitorMigrateToHost(qemuMonitorPtr mon, unsigned int flags, + const char *proto, const char *hostname, int port) { @@ -2113,7 +2114,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 82e6ae2..d722e12 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -429,6 +429,7 @@ int qemuMonitorMigrateToFd(qemuMonitorPtr mon, int qemuMonitorMigrateToHost(qemuMonitorPtr mon, unsigned int flags, + const char *proto, const char *hostname, int port); -- 1.7.10.4

On 07/26/2013 02:27 PM, Jiri Denemark wrote: Acknowledged. How would we detect without using the QEMU version? This feature doesn't have any new command-line arguments (except for -incoming ....) Laziness on my part - It was never implemented because QEMU's parsing of the "[::]" brackets is hard-coded for TCP/IP. I'll submit a patch to break this out on qemu-devel. Having a conditional check only for 'rdma' is what I was trying to avoid. Wouldn't it be better to have both options available? Having both choices is kind of nice - and it's unlikely that users will stop breaking the habit for a while. Understood =)