On 07/26/2013 02:27 PM, Jiri Denemark wrote:
On Fri, Jul 26, 2013 at 13:47:44 -0400, mrhi...@linux.vnet.ibm.com wrote:From: "Michael R. Hines" <mrhi...@us.ibm.com>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.This does not prevent us from calling the protocol "rdma" right away and possibly translating it to "x-rdma". However, I don't think we actually want to commit patches for rdma migration before QEMU changes the name to "rdma".
Acknowledged.
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;This is wrong. First, you're adding this into a totally wrong place and second, we need a better detection which is not based on qemu version.
How would we detect without using the QEMU version? This feature doesn't have any new command-line arguments (except for -incoming ....)
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 {Is there any reason why RDMA migration does not work over IPv6?
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.
@@ -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; }Just because we did the mistake with tcp protocol we don't have to repeat it with rdma. Just change the rdma protocol to always be well-formed, i.e., rdma://host
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.
@@ -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); + } +You're not supposed to check if a variable you're about to free is non-NULL. Running make syntax-check would have warned you.
Understood =) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
