[libvirt] [PATCH] qemu: fix well-formed migration URI formatting

When adding an automatically allocated port to a well-formed migration URI, keep it well-formed: tcp://1.2.3.4/ -> tcp://1.2.3.4/:12345 # wrong tcp://1.2.3.4/ -> tcp://1.2.3.4:12345/ # fixed tcp://1.2.3.4 -> tcp://1.2.3.4:12345 # still works tcp:1.2.3.4 -> tcp:1.2.3.4:12345 # still works (old syntax) Signed-off-by: Michael Chapman <mike@very.puzzling.org> --- src/qemu/qemu_migration.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 0439ba4..cb59620 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2535,6 +2535,7 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver, char *uri_str = NULL; int ret = -1; virURIPtr uri = NULL; + bool well_formed_uri = true; VIR_DEBUG("driver=%p, dconn=%p, cookiein=%s, cookieinlen=%d, " "cookieout=%p, cookieoutlen=%p, uri_in=%s, uri_out=%p, " @@ -2597,6 +2598,7 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver, /* Convert uri_in to well-formed URI with // after tcp: */ if (!(STRPREFIX(uri_in, "tcp://"))) { + well_formed_uri = false; if (virAsprintf(&uri_str, "tcp://%s", p) < 0) goto cleanup; } @@ -2626,9 +2628,17 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver, goto cleanup; } - /* Caller frees */ - if (virAsprintf(uri_out, "%s:%d", uri_in, port) < 0) - goto cleanup; + if (well_formed_uri) { + uri->port = port; + + /* Caller frees */ + if (!(*uri_out = virURIFormat(uri))) + goto cleanup; + } else { + /* Caller frees */ + if (virAsprintf(uri_out, "%s:%d", uri_in, port) < 0) + goto cleanup; + } } else { port = uri->port; -- 1.8.3.1

On Wed, 23 Oct 2013, Michael Chapman wrote:
When adding an automatically allocated port to a well-formed migration URI, keep it well-formed:
tcp://1.2.3.4/ -> tcp://1.2.3.4/:12345 # wrong tcp://1.2.3.4/ -> tcp://1.2.3.4:12345/ # fixed tcp://1.2.3.4 -> tcp://1.2.3.4:12345 # still works tcp:1.2.3.4 -> tcp:1.2.3.4:12345 # still works (old syntax)
Hi, I was wondering if anyone has had a chance to look at this patch. It's pretty straight-forward, just making sure that qemuMigrationPrepareDirect doesn't return a malformed migration URI when the one supplied doesn't have an explicit port number. without this patch, if the user supplies a well-formed tcp://<ip>/ migration URI without a port, QEMU on the source side of the migration is told to connect to the target on port 0, which obviously doesn't work too well. Regards, Michael

On 10/22/2013 10:15 PM, Michael Chapman wrote:
When adding an automatically allocated port to a well-formed migration URI, keep it well-formed:
tcp://1.2.3.4/ -> tcp://1.2.3.4/:12345 # wrong tcp://1.2.3.4/ -> tcp://1.2.3.4:12345/ # fixed tcp://1.2.3.4 -> tcp://1.2.3.4:12345 # still works tcp:1.2.3.4 -> tcp:1.2.3.4:12345 # still works (old syntax)
Signed-off-by: Michael Chapman <mike@very.puzzling.org> --- src/qemu/qemu_migration.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-)
Apologies for the slow review time. ACK and pushed. A testsuite addition would also be nice, to prove we don't regress... -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Tue, 29 Oct 2013, Eric Blake wrote:
Apologies for the slow review time.
ACK and pushed. A testsuite addition would also be nice, to prove we don't regress...
Thanks! I did look into adding this to the testsuite, but as far as I can tell we don't have anything there yet that runs through the migration phases. For a proper test I'm guessing we'd need to mock the QEMU binary itself, since the prepare phase wants to start it. I suppose it might be possible just to run through an offline migration, but as a migration URI makes no sense for offline migration it's not really a fair test. - Michael
participants (2)
-
Eric Blake
-
Michael Chapman