[libvirt] [PATCHv2 0/4] qemu: Allow users specify -incoming listen address

v2 for my previous attempt: https://www.redhat.com/archives/libvir-list/2013-October/msg00400.html diff to v1: -Check if listenAddress is a numeric IPv4 or IPv6 prior parsing it -enclose all IPv6 addresses in angle brackets 1/4 has been ACKed already Michal Privoznik (4): Migration: Introduce VIR_MIGRATE_PARAM_LISTEN_ADDRESS virsocket: Introduce virSocketAddrIsWildcard qemu: Implement support for VIR_MIGRATE_PARAM_LISTEN_ADDRESS qemu_conf: Introduce "migration_address" include/libvirt/libvirt.h.in | 10 ++++ src/libvirt_private.syms | 1 + src/qemu/libvirtd_qemu.aug | 3 + src/qemu/qemu.conf | 6 ++ src/qemu/qemu_conf.c | 2 + src/qemu/qemu_conf.h | 3 + src/qemu/qemu_driver.c | 33 ++++++++--- src/qemu/qemu_migration.c | 113 +++++++++++++++++++++++++------------ src/qemu/qemu_migration.h | 13 +++-- src/qemu/test_libvirtd_qemu.aug.in | 1 + src/util/virsocketaddr.c | 71 +++++++++++++++++------ src/util/virsocketaddr.h | 2 + tests/sockettest.c | 29 ++++++++++ tools/virsh-domain.c | 11 ++++ tools/virsh.pod | 10 +++- 15 files changed, 241 insertions(+), 67 deletions(-) -- 1.8.1.5

The parameter allows overriding default listen address for '-incoming' cmd line argument on destination. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- ACKed already, I'm sending it just for a completeness-sake. include/libvirt/libvirt.h.in | 10 ++++++++++ tools/virsh-domain.c | 11 +++++++++++ tools/virsh.pod | 10 ++++++++-- 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 83c219e..820e8ce 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1270,6 +1270,16 @@ typedef enum { */ #define VIR_MIGRATE_PARAM_GRAPHICS_URI "graphics_uri" +/** + * VIR_MIGRATE_PARAM_LISTEN_ADDRESS: + * + * virDomainMigrate* params field: The listen address that hypervisor on the + * destination side should bind to for incoming migration. Both, IPv4 and IPv6 + * addresses are accepted as well as hostnames (the resolving is done on + * destination). Some hypervisors do not support this feature an will return an + * error if this field is used. + */ +#define VIR_MIGRATE_PARAM_LISTEN_ADDRESS "listen_address" /* Domain migration. */ virDomainPtr virDomainMigrate (virDomainPtr domain, virConnectPtr dconn, diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 42c9920..6d241db 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -8550,6 +8550,10 @@ static const vshCmdOptDef opts_migrate[] = { .type = VSH_OT_DATA, .help = N_("graphics URI to be used for seamless graphics migration") }, + {.name = "listen-address", + .type = VSH_OT_DATA, + .help = N_("listen address that destination should bind to for incoming migration") + }, {.name = "dname", .type = VSH_OT_DATA, .help = N_("rename to new name during migration (if supported)") @@ -8606,6 +8610,13 @@ doMigrate(void *opaque) VIR_MIGRATE_PARAM_GRAPHICS_URI, opt) < 0) goto save_error; + if (vshCommandOptStringReq(ctl, cmd, "listen-address", &opt) < 0) + goto out; + if (opt && + virTypedParamsAddString(¶ms, &nparams, &maxparams, + VIR_MIGRATE_PARAM_LISTEN_ADDRESS, opt) < 0) + goto save_error; + if (vshCommandOptStringReq(ctl, cmd, "dname", &opt) < 0) goto out; if (opt && diff --git a/tools/virsh.pod b/tools/virsh.pod index e12a800..d555565 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1081,8 +1081,8 @@ stats. [I<--persistent>] [I<--undefinesource>] [I<--suspend>] [I<--copy-storage-all>] [I<--copy-storage-inc>] [I<--change-protection>] [I<--unsafe>] [I<--verbose>] [I<--compressed>] [I<--abort-on-error>] -I<domain> I<desturi> [I<migrateuri>] [I<graphicsuri>] [I<dname>] -[I<--timeout> B<seconds>] [I<--xml> B<file>] +I<domain> I<desturi> [I<migrateuri>] [I<graphicsuri>] [I<listen-address>] +[I<dname>] [I<--timeout> B<seconds>] [I<--xml> B<file>] Migrate domain to another host. Add I<--live> for live migration; <--p2p> for peer-2-peer migration; I<--direct> for direct migration; or I<--tunnelled> @@ -1189,6 +1189,12 @@ specific parameters separated by '&'. Currently recognized parameters are spice://target.host.com:1234/?tlsPort=4567 +Optional I<listen-address> sets the listen address that hypervisor on the +destination side should bind to for incoming migration. Both, IPv4 and IPv6 +addresses are accepted as well as hostnames (the resolving is done on +destination). Some hypervisors do not support this feature an will return an +error if this parameter is used. + =item B<migrate-setmaxdowntime> I<domain> I<downtime> Set maximum tolerable downtime for a domain which is being live-migrated to -- 1.8.1.5

On 10/09/2013 03:32 PM, Michal Privoznik wrote:
--- a/tools/virsh.pod +++ b/tools/virsh.pod
+Optional I<listen-address> sets the listen address that hypervisor on the +destination side should bind to for incoming migration. Both, IPv4 and IPv6 Redundant comma. --^
+addresses are accepted as well as hostnames (the resolving is done on +destination). Some hypervisors do not support this feature an will return an +error if this parameter is used. + =item B<migrate-setmaxdowntime> I<domain> I<downtime>
Set maximum tolerable downtime for a domain which is being live-migrated to

On 10/10/2013 07:52 AM, Ján Tomko wrote:
On 10/09/2013 03:32 PM, Michal Privoznik wrote:
--- a/tools/virsh.pod +++ b/tools/virsh.pod
+Optional I<listen-address> sets the listen address that hypervisor on the +destination side should bind to for incoming migration. Both, IPv4 and IPv6 Redundant comma. --^
+addresses are accepted as well as hostnames (the resolving is done on +destination). Some hypervisors do not support this feature an will return an
s/an will/and will/
+error if this parameter is used. + =item B<migrate-setmaxdowntime> I<domain> I<downtime>
Set maximum tolerable downtime for a domain which is being live-migrated to
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

This function takes exactly one argument: an address to check. It returns true, iff the address is an IPv4 or IPv6 address in numeric format, false if otherwise (e.g. for "examplehost"). Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virsocketaddr.c | 71 +++++++++++++++++++++++++++++++++++++----------- src/util/virsocketaddr.h | 2 ++ tests/sockettest.c | 29 ++++++++++++++++++++ 4 files changed, 87 insertions(+), 16 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index fe40834..54ef9a7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1682,6 +1682,7 @@ virSocketAddrGetIpPrefix; virSocketAddrGetPort; virSocketAddrGetRange; virSocketAddrIsNetmask; +virSocketAddrIsNumeric; virSocketAddrIsPrivate; virSocketAddrIsWildcard; virSocketAddrMask; diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c index 3e01baf..ddb3058 100644 --- a/src/util/virsocketaddr.c +++ b/src/util/virsocketaddr.c @@ -71,6 +71,35 @@ static int virSocketAddrGetIPv6Addr(virSocketAddrPtr addr, virSocketAddrIPv6Ptr return 0; } +static int +virSocketAddrParseInternal(struct addrinfo **res, + const char *val, + int family, + bool reportError) +{ + struct addrinfo hints; + int err; + + if (val == NULL) { + virReportError(VIR_ERR_INVALID_ARG, "%s", _("Missing address")); + return -1; + } + + memset(&hints, 0, sizeof(hints)); + hints.ai_family = family; + hints.ai_flags = AI_NUMERICHOST; + if ((err = getaddrinfo(val, NULL, &hints, res)) != 0) { + if (reportError) + virReportError(VIR_ERR_SYSTEM_ERROR, + _("Cannot parse socket address '%s': %s"), + val, gai_strerror(err)); + + return -1; + } + + return 0; +} + /** * virSocketAddrParse: * @val: a numeric network address IPv4 or IPv6 @@ -84,24 +113,10 @@ static int virSocketAddrGetIPv6Addr(virSocketAddrPtr addr, virSocketAddrIPv6Ptr */ int virSocketAddrParse(virSocketAddrPtr addr, const char *val, int family) { int len; - struct addrinfo hints; - struct addrinfo *res = NULL; - int err; + struct addrinfo *res; - if (val == NULL) { - virReportError(VIR_ERR_INVALID_ARG, "%s", _("Missing address")); + if (virSocketAddrParseInternal(&res, val, family, true) < 0) return -1; - } - - memset(&hints, 0, sizeof(hints)); - hints.ai_family = family; - hints.ai_flags = AI_NUMERICHOST; - if ((err = getaddrinfo(val, NULL, &hints, &res)) != 0) { - virReportError(VIR_ERR_SYSTEM_ERROR, - _("Cannot parse socket address '%s': %s"), - val, gai_strerror(err)); - return -1; - } if (res == NULL) { virReportError(VIR_ERR_SYSTEM_ERROR, @@ -824,3 +839,27 @@ virSocketAddrGetIpPrefix(const virSocketAddrPtr address, */ return 0; } +/** + * virSocketAddrIsNumeric: + * @address: address to check + * + * Check if passed address is an IP address in numeric format. For + * instance, for 0.0.0.0 true is returned, for 'examplehost" + * false is returned. + * + * Returns: true iff @address is an IP address, + * false otherwise + */ +bool +virSocketAddrIsNumeric(const char *address) +{ + struct addrinfo *res; + unsigned short family; + + if (virSocketAddrParseInternal(&res, address, AF_UNSPEC, false) < 0) + return false; + + family = res->ai_addr->sa_family; + freeaddrinfo(res); + return family == AF_INET || family == AF_INET6; +} diff --git a/src/util/virsocketaddr.h b/src/util/virsocketaddr.h index b28fe6c..d844914 100644 --- a/src/util/virsocketaddr.h +++ b/src/util/virsocketaddr.h @@ -124,4 +124,6 @@ bool virSocketAddrEqual(const virSocketAddrPtr s1, bool virSocketAddrIsPrivate(const virSocketAddrPtr addr); bool virSocketAddrIsWildcard(const virSocketAddrPtr addr); + +bool virSocketAddrIsNumeric(const char *address); #endif /* __VIR_SOCKETADDR_H__ */ diff --git a/tests/sockettest.c b/tests/sockettest.c index 4b38cdf..f98955d 100644 --- a/tests/sockettest.c +++ b/tests/sockettest.c @@ -174,6 +174,21 @@ static int testWildcardHelper(const void *opaque) return testWildcard(data->addr, data->pass); } +struct testIsNumericData { + const char *addr; + bool pass; +}; + +static int +testIsNumericHelper(const void *opaque) +{ + const struct testIsNumericData *data = opaque; + + if (virSocketAddrIsNumeric(data->addr)) + return data->pass ? 0 : -1; + return data->pass ? -1 : 0; +} + static int mymain(void) { @@ -246,6 +261,14 @@ mymain(void) ret = -1; \ } while (0) +#define DO_TEST_IS_NUMERIC(addr, pass) \ + do { \ + struct testIsNumericData data = { addr, pass}; \ + if (virtTestRun("Test isNumeric " addr, \ + testIsNumericHelper, &data) < 0) \ + ret = -1; \ + } while (0) + DO_TEST_PARSE_AND_FORMAT("127.0.0.1", AF_UNSPEC, true); DO_TEST_PARSE_AND_FORMAT("127.0.0.1", AF_INET, true); @@ -307,6 +330,12 @@ mymain(void) DO_TEST_WILDCARD("1", false); DO_TEST_WILDCARD("0.1", false); + DO_TEST_IS_NUMERIC("0.0.0.0", true); + DO_TEST_IS_NUMERIC("::", true); + DO_TEST_IS_NUMERIC("1", true); + DO_TEST_IS_NUMERIC("::ffff", true); + DO_TEST_IS_NUMERIC("examplehost", false); + return ret==0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 1.8.1.5

On 10/09/2013 03:32 PM, Michal Privoznik wrote:
This function takes exactly one argument: an address to check. It returns true, iff the address is an IPv4 or IPv6 address in numeric
Why use "iff" if you still need to say what happens otherwise? :)
format, false if otherwise (e.g. for "examplehost").
s/ if//
@@ -824,3 +839,27 @@ virSocketAddrGetIpPrefix(const virSocketAddrPtr address, */ return 0; }
Missing space.
+/** + * virSocketAddrIsNumeric: + * @address: address to check + * + * Check if passed address is an IP address in numeric format. For + * instance, for 0.0.0.0 true is returned, for 'examplehost" + * false is returned. + * + * Returns: true iff @address is an IP address,
See my comment for the commit message.
+ * false otherwise + */ +bool +virSocketAddrIsNumeric(const char *address) +{ + struct addrinfo *res; + unsigned short family; + + if (virSocketAddrParseInternal(&res, address, AF_UNSPEC, false) < 0) + return false; + + family = res->ai_addr->sa_family;
Is this any different than using int res->ai_family?
+ freeaddrinfo(res); + return family == AF_INET || family == AF_INET6; +}
ACK

Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 26 +++++++---- src/qemu/qemu_migration.c | 113 +++++++++++++++++++++++++++++++--------------- src/qemu/qemu_migration.h | 13 ++++-- 3 files changed, 103 insertions(+), 49 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c71aecc..e12a1de 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10038,7 +10038,7 @@ qemuDomainMigratePrepare2(virConnectPtr dconn, ret = qemuMigrationPrepareDirect(driver, dconn, NULL, 0, NULL, NULL, /* No cookies */ uri_in, uri_out, - &def, origname, flags); + &def, origname, NULL, flags); cleanup: VIR_FREE(origname); @@ -10089,7 +10089,8 @@ qemuDomainMigratePerform(virDomainPtr dom, * Consume any cookie we were able to decode though */ ret = qemuMigrationPerform(driver, dom->conn, vm, - NULL, dconnuri, uri, NULL, cookie, cookielen, + NULL, dconnuri, uri, NULL, NULL, + cookie, cookielen, NULL, NULL, /* No output cookies in v2 */ flags, dname, resource, false); @@ -10243,7 +10244,7 @@ qemuDomainMigratePrepare3(virConnectPtr dconn, cookiein, cookieinlen, cookieout, cookieoutlen, uri_in, uri_out, - &def, origname, flags); + &def, origname, NULL, flags); cleanup: VIR_FREE(origname); @@ -10267,6 +10268,7 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn, const char *dom_xml = NULL; const char *dname = NULL; const char *uri_in = NULL; + const char *listenAddress = NULL; char *origname = NULL; int ret = -1; @@ -10282,7 +10284,10 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn, &dname) < 0 || virTypedParamsGetString(params, nparams, VIR_MIGRATE_PARAM_URI, - &uri_in) < 0) + &uri_in) < 0 || + virTypedParamsGetString(params, nparams, + VIR_MIGRATE_PARAM_LISTEN_ADDRESS, + &listenAddress) < 0) return -1; if (flags & VIR_MIGRATE_TUNNELLED) { @@ -10305,7 +10310,7 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn, cookiein, cookieinlen, cookieout, cookieoutlen, uri_in, uri_out, - &def, origname, flags); + &def, origname, listenAddress, flags); cleanup: VIR_FREE(origname); @@ -10437,7 +10442,8 @@ qemuDomainMigratePerform3(virDomainPtr dom, } return qemuMigrationPerform(driver, dom->conn, vm, xmlin, - dconnuri, uri, NULL, cookiein, cookieinlen, + dconnuri, uri, NULL, NULL, + cookiein, cookieinlen, cookieout, cookieoutlen, flags, dname, resource, true); } @@ -10459,6 +10465,7 @@ qemuDomainMigratePerform3Params(virDomainPtr dom, const char *dname = NULL; const char *uri = NULL; const char *graphicsuri = NULL; + const char *listenAddress = NULL; unsigned long long bandwidth = 0; virCheckFlags(QEMU_MIGRATION_FLAGS, -1); @@ -10479,7 +10486,10 @@ qemuDomainMigratePerform3Params(virDomainPtr dom, &bandwidth) < 0 || virTypedParamsGetString(params, nparams, VIR_MIGRATE_PARAM_GRAPHICS_URI, - &graphicsuri) < 0) + &graphicsuri) < 0 || + virTypedParamsGetString(params, nparams, + VIR_MIGRATE_PARAM_LISTEN_ADDRESS, + &listenAddress) < 0) return -1; if (!(vm = qemuDomObjFromDomain(dom))) @@ -10491,7 +10501,7 @@ qemuDomainMigratePerform3Params(virDomainPtr dom, } return qemuMigrationPerform(driver, dom->conn, vm, dom_xml, - dconnuri, uri, graphicsuri, + dconnuri, uri, graphicsuri, listenAddress, cookiein, cookieinlen, cookieout, cookieoutlen, flags, dname, bandwidth, true); } diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 3a1aab7..2140073 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1097,12 +1097,6 @@ qemuMigrationStartNBDServer(virQEMUDriverPtr driver, unsigned short port = 0; char *diskAlias = NULL; size_t i; - const char *host; - - if (STREQ(listenAddr, "[::]")) - host = "::"; - else - host = listenAddr; for (i = 0; i < vm->def->ndisks; i++) { virDomainDiskDefPtr disk = vm->def->disks[i]; @@ -1122,7 +1116,7 @@ qemuMigrationStartNBDServer(virQEMUDriverPtr driver, if (!port && ((virPortAllocatorAcquire(driver->remotePorts, &port) < 0) || - (qemuMonitorNBDServerStart(priv->mon, host, port) < 0))) { + (qemuMonitorNBDServerStart(priv->mon, listenAddr, port) < 0))) { qemuDomainObjExitMonitor(driver, vm); goto cleanup; } @@ -2163,6 +2157,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, const char *origname, virStreamPtr st, unsigned int port, + const char *listenAddress, unsigned long flags) { virDomainObjPtr vm = NULL; @@ -2176,7 +2171,6 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, char *xmlout = NULL; unsigned int cookieFlags; virCapsPtr caps = NULL; - const char *listenAddr = NULL; char *migrateFrom = NULL; bool abort_on_error = !!(flags & VIR_MIGRATE_ABORT_ON_ERROR); @@ -2260,31 +2254,66 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, if (VIR_STRDUP(migrateFrom, "stdio") < 0) goto cleanup; } else { + virSocketAddr listenAddressSocket; + bool encloseAddress = false; + bool hostIPv6Capable = false; + bool qemuIPv6Capable = false; virQEMUCapsPtr qemuCaps = NULL; struct addrinfo *info = NULL; struct addrinfo hints = { .ai_flags = AI_ADDRCONFIG, .ai_socktype = SOCK_STREAM }; + if (getaddrinfo("::", NULL, &hints, &info) == 0) { + freeaddrinfo(info); + hostIPv6Capable = true; + } if (!(qemuCaps = virQEMUCapsCacheLookupCopy(driver->qemuCapsCache, (*def)->emulator))) goto cleanup; - /* Listen on :: instead of 0.0.0.0 if QEMU understands it - * and there is at least one IPv6 address configured - */ - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_IPV6_MIGRATION) && - getaddrinfo("::", NULL, &hints, &info) == 0) { - freeaddrinfo(info); - listenAddr = "[::]"; + qemuIPv6Capable = virQEMUCapsGet(qemuCaps, QEMU_CAPS_IPV6_MIGRATION); + virObjectUnref(qemuCaps); + + if (listenAddress) { + if (virSocketAddrIsNumeric(listenAddress)) { + /* listenAddress is numeric IPv4 or IPv6 */ + if (virSocketAddrParse(&listenAddressSocket, listenAddress, AF_UNSPEC) < 0) + goto cleanup; + + /* address parsed successfully */ + if (VIR_SOCKET_ADDR_IS_FAMILY(&listenAddressSocket, AF_INET6)) { + if (!qemuIPv6Capable) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("qemu isn't capable of IPv6")); + goto cleanup; + } + if (!hostIPv6Capable) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("host isn't capable of IPv6")); + goto cleanup; + } + } + + /* IPv6 address must be enclosed in angle brackets on the cmd line */ + encloseAddress = true; + } else { + /* listenAddress is a hostname */ + } } else { - listenAddr = "0.0.0.0"; + /* Listen on :: instead of 0.0.0.0 if QEMU understands it + * and there is at least one IPv6 address configured + */ + listenAddress = qemuIPv6Capable && hostIPv6Capable ? + encloseAddress = true, "::" : "0.0.0.0"; } - virObjectUnref(qemuCaps); - /* QEMU will be started with -incoming [::]:port - * or -incoming 0.0.0.0:port + /* QEMU will be started with -incoming [<IPv6 addr>]:port, + * -incoming <IPv4 addr>:port or -incoming <hostname>:port */ - if (virAsprintf(&migrateFrom, "tcp:%s:%d", listenAddr, port) < 0) + if ((encloseAddress && + virAsprintf(&migrateFrom, "tcp:[%s]:%d", listenAddress, port) < 0) || + (!encloseAddress && + virAsprintf(&migrateFrom, "tcp:%s:%d", listenAddress, port) < 0)) goto cleanup; } @@ -2371,7 +2400,7 @@ done: if (mig->nbd && flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC) && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NBD_SERVER)) { - if (qemuMigrationStartNBDServer(driver, vm, listenAddr) < 0) { + if (qemuMigrationStartNBDServer(driver, vm, listenAddress) < 0) { /* error already reported */ goto endjob; } @@ -2475,7 +2504,7 @@ qemuMigrationPrepareTunnel(virQEMUDriverPtr driver, ret = qemuMigrationPrepareAny(driver, dconn, cookiein, cookieinlen, cookieout, cookieoutlen, def, origname, - st, 0, flags); + st, 0, NULL, flags); return ret; } @@ -2491,6 +2520,7 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver, char **uri_out, virDomainDefPtr *def, const char *origname, + const char *listenAddress, unsigned long flags) { static int port = 0; @@ -2596,7 +2626,7 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver, ret = qemuMigrationPrepareAny(driver, dconn, cookiein, cookieinlen, cookieout, cookieoutlen, def, origname, - NULL, this_port, flags); + NULL, this_port, listenAddress, flags); cleanup: virURIFree(uri); VIR_FREE(hostname); @@ -3600,6 +3630,7 @@ doPeer2PeerMigrate3(virQEMUDriverPtr driver, const char *dname, const char *uri, const char *graphicsuri, + const char *listenAddress, unsigned long long bandwidth, bool useParams, unsigned long flags) @@ -3621,11 +3652,11 @@ doPeer2PeerMigrate3(virQEMUDriverPtr driver, int maxparams = 0; VIR_DEBUG("driver=%p, sconn=%p, dconn=%p, dconnuri=%s, vm=%p, xmlin=%s, " - "dname=%s, uri=%s, graphicsuri=%s, bandwidth=%llu, " - "useParams=%d, flags=%lx", + "dname=%s, uri=%s, graphicsuri=%s, listenAddress=%s, " + "bandwidth=%llu, useParams=%d, flags=%lx", driver, sconn, dconn, NULLSTR(dconnuri), vm, NULLSTR(xmlin), - NULLSTR(dname), NULLSTR(uri), NULLSTR(graphicsuri), bandwidth, - useParams, flags); + NULLSTR(dname), NULLSTR(uri), NULLSTR(graphicsuri), + NULLSTR(listenAddress), bandwidth, useParams, flags); /* Unlike the virDomainMigrateVersion3 counterpart, we don't need * to worry about auto-setting the VIR_MIGRATE_CHANGE_PROTECTION @@ -3663,6 +3694,11 @@ doPeer2PeerMigrate3(virQEMUDriverPtr driver, VIR_MIGRATE_PARAM_GRAPHICS_URI, graphicsuri) < 0) goto cleanup; + if (listenAddress && + virTypedParamsAddString(¶ms, &nparams, &maxparams, + VIR_MIGRATE_PARAM_LISTEN_ADDRESS, + listenAddress) < 0) + goto cleanup; } if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) @@ -3867,6 +3903,7 @@ static int doPeer2PeerMigrate(virQEMUDriverPtr driver, const char *dconnuri, const char *uri, const char *graphicsuri, + const char *listenAddress, unsigned long flags, const char *dname, unsigned long resource, @@ -3881,10 +3918,11 @@ static int doPeer2PeerMigrate(virQEMUDriverPtr driver, bool useParams; VIR_DEBUG("driver=%p, sconn=%p, vm=%p, xmlin=%s, dconnuri=%s, " - "uri=%s, graphicsuri=%s, flags=%lx, dname=%s, resource=%lu", + "uri=%s, graphicsuri=%s, listenAddress=%s, flags=%lx, " + "dname=%s, resource=%lu", driver, sconn, vm, NULLSTR(xmlin), NULLSTR(dconnuri), - NULLSTR(uri), NULLSTR(graphicsuri), flags, NULLSTR(dname), - resource); + NULLSTR(uri), NULLSTR(graphicsuri), NULLSTR(listenAddress), + flags, NULLSTR(dname), resource); /* the order of operations is important here; we make sure the * destination side is completely setup before we touch the source @@ -3959,8 +3997,8 @@ static int doPeer2PeerMigrate(virQEMUDriverPtr driver, if (*v3proto) { ret = doPeer2PeerMigrate3(driver, sconn, dconn, dconnuri, vm, xmlin, - dname, uri, graphicsuri, resource, - useParams, flags); + dname, uri, graphicsuri, listenAddress, + resource, useParams, flags); } else { ret = doPeer2PeerMigrate2(driver, sconn, dconn, vm, dconnuri, flags, dname, resource); @@ -3994,6 +4032,7 @@ qemuMigrationPerformJob(virQEMUDriverPtr driver, const char *uri, const char *graphicsuri, const char *cookiein, + const char *listenAddress, int cookieinlen, char **cookieout, int *cookieoutlen, @@ -4028,8 +4067,8 @@ qemuMigrationPerformJob(virQEMUDriverPtr driver, if ((flags & (VIR_MIGRATE_TUNNELLED | VIR_MIGRATE_PEER2PEER))) { ret = doPeer2PeerMigrate(driver, conn, vm, xmlin, - dconnuri, uri, graphicsuri, flags, dname, - resource, &v3proto); + dconnuri, uri, graphicsuri, listenAddress, + flags, dname, resource, &v3proto); } else { qemuMigrationJobSetPhase(driver, vm, QEMU_MIGRATION_PHASE_PERFORM2); ret = doNativeMigrate(driver, vm, uri, cookiein, cookieinlen, @@ -4194,6 +4233,7 @@ qemuMigrationPerform(virQEMUDriverPtr driver, const char *dconnuri, const char *uri, const char *graphicsuri, + const char *listenAddress, const char *cookiein, int cookieinlen, char **cookieout, @@ -4220,7 +4260,8 @@ qemuMigrationPerform(virQEMUDriverPtr driver, } return qemuMigrationPerformJob(driver, conn, vm, xmlin, dconnuri, uri, - graphicsuri, cookiein, cookieinlen, + graphicsuri, listenAddress, + cookiein, cookieinlen, cookieout, cookieoutlen, flags, dname, resource, v3proto); } else { @@ -4238,7 +4279,7 @@ qemuMigrationPerform(virQEMUDriverPtr driver, flags, resource); } else { return qemuMigrationPerformJob(driver, conn, vm, xmlin, dconnuri, - uri, graphicsuri, + uri, graphicsuri, listenAddress, cookiein, cookieinlen, cookieout, cookieoutlen, flags, dname, resource, v3proto); diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index 4af5aed..cafa2a2 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -43,11 +43,12 @@ /* All supported migration parameters and their types. */ # define QEMU_MIGRATION_PARAMETERS \ - VIR_MIGRATE_PARAM_URI, VIR_TYPED_PARAM_STRING, \ - VIR_MIGRATE_PARAM_DEST_NAME, VIR_TYPED_PARAM_STRING, \ - VIR_MIGRATE_PARAM_DEST_XML, VIR_TYPED_PARAM_STRING, \ - VIR_MIGRATE_PARAM_BANDWIDTH, VIR_TYPED_PARAM_ULLONG, \ - VIR_MIGRATE_PARAM_GRAPHICS_URI, VIR_TYPED_PARAM_STRING, \ + VIR_MIGRATE_PARAM_URI, VIR_TYPED_PARAM_STRING, \ + VIR_MIGRATE_PARAM_DEST_NAME, VIR_TYPED_PARAM_STRING, \ + VIR_MIGRATE_PARAM_DEST_XML, VIR_TYPED_PARAM_STRING, \ + VIR_MIGRATE_PARAM_BANDWIDTH, VIR_TYPED_PARAM_ULLONG, \ + VIR_MIGRATE_PARAM_GRAPHICS_URI, VIR_TYPED_PARAM_STRING, \ + VIR_MIGRATE_PARAM_LISTEN_ADDRESS, VIR_TYPED_PARAM_STRING, \ NULL @@ -124,6 +125,7 @@ int qemuMigrationPrepareDirect(virQEMUDriverPtr driver, char **uri_out, virDomainDefPtr *def, const char *origname, + const char *listenAddress, unsigned long flags); int qemuMigrationPerform(virQEMUDriverPtr driver, @@ -133,6 +135,7 @@ int qemuMigrationPerform(virQEMUDriverPtr driver, const char *dconnuri, const char *uri, const char *graphicsuri, + const char *listenAddress, const char *cookiein, int cookieinlen, char **cookieout, -- 1.8.1.5

On 10/09/2013 03:32 PM, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 26 +++++++---- src/qemu/qemu_migration.c | 113 +++++++++++++++++++++++++++++++--------------- src/qemu/qemu_migration.h | 13 ++++-- 3 files changed, 103 insertions(+), 49 deletions(-)
@@ -2260,31 +2254,66 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, if (VIR_STRDUP(migrateFrom, "stdio") < 0) goto cleanup; } else { + virSocketAddr listenAddressSocket; + bool encloseAddress = false; + bool hostIPv6Capable = false; + bool qemuIPv6Capable = false; virQEMUCapsPtr qemuCaps = NULL; struct addrinfo *info = NULL; struct addrinfo hints = { .ai_flags = AI_ADDRCONFIG, .ai_socktype = SOCK_STREAM };
+ if (getaddrinfo("::", NULL, &hints, &info) == 0) { + freeaddrinfo(info); + hostIPv6Capable = true; + } if (!(qemuCaps = virQEMUCapsCacheLookupCopy(driver->qemuCapsCache, (*def)->emulator))) goto cleanup;
- /* Listen on :: instead of 0.0.0.0 if QEMU understands it - * and there is at least one IPv6 address configured - */ - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_IPV6_MIGRATION) && - getaddrinfo("::", NULL, &hints, &info) == 0) { - freeaddrinfo(info); - listenAddr = "[::]"; + qemuIPv6Capable = virQEMUCapsGet(qemuCaps, QEMU_CAPS_IPV6_MIGRATION); + virObjectUnref(qemuCaps); + + if (listenAddress) { + if (virSocketAddrIsNumeric(listenAddress)) { + /* listenAddress is numeric IPv4 or IPv6 */ + if (virSocketAddrParse(&listenAddressSocket, listenAddress, AF_UNSPEC) < 0) + goto cleanup; + + /* address parsed successfully */ + if (VIR_SOCKET_ADDR_IS_FAMILY(&listenAddressSocket, AF_INET6)) { + if (!qemuIPv6Capable) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("qemu isn't capable of IPv6")); + goto cleanup; + } + if (!hostIPv6Capable) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("host isn't capable of IPv6")); + goto cleanup; + } + } + + /* IPv6 address must be enclosed in angle brackets on the cmd line */ + encloseAddress = true;
This sets it for IPv4 as well. Also s/ angle// (and maybe 'escape' would be better than 'enclose')
+ } else { + /* listenAddress is a hostname */ + } } else { - listenAddr = "0.0.0.0"; + /* Listen on :: instead of 0.0.0.0 if QEMU understands it + * and there is at least one IPv6 address configured + */ + listenAddress = qemuIPv6Capable && hostIPv6Capable ? + encloseAddress = true, "::" : "0.0.0.0"; } - virObjectUnref(qemuCaps);
- /* QEMU will be started with -incoming [::]:port - * or -incoming 0.0.0.0:port + /* QEMU will be started with -incoming [<IPv6 addr>]:port, + * -incoming <IPv4 addr>:port or -incoming <hostname>:port */ - if (virAsprintf(&migrateFrom, "tcp:%s:%d", listenAddr, port) < 0) + if ((encloseAddress && + virAsprintf(&migrateFrom, "tcp:[%s]:%d", listenAddress, port) < 0) || + (!encloseAddress && + virAsprintf(&migrateFrom, "tcp:%s:%d", listenAddress, port) < 0)) goto cleanup; }
ACK with that fixed

This configuration knob is there to override default listen address for -incoming for all qemu domains. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/libvirtd_qemu.aug | 3 +++ src/qemu/qemu.conf | 6 ++++++ src/qemu/qemu_conf.c | 2 ++ src/qemu/qemu_conf.h | 3 +++ src/qemu/qemu_driver.c | 7 +++++++ src/qemu/test_libvirtd_qemu.aug.in | 1 + 6 files changed, 22 insertions(+) diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index cd13d53..591d78d 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -78,6 +78,8 @@ module Libvirtd_qemu = | int_entry "keepalive_interval" | int_entry "keepalive_count" + let network_entry = str_entry "migration_address" + (* Each entry in the config is one of the following ... *) let entry = vnc_entry | spice_entry @@ -88,6 +90,7 @@ module Libvirtd_qemu = | process_entry | device_entry | rpc_entry + | network_entry let comment = [ label "#comment" . del /#[ \t]*/ "# " . store /([^ \t\n][^\n]*)?/ . del /\n/ "\n" ] let empty = [ label "#empty" . eol ] diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 5fd6263..4403aaf 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -424,3 +424,9 @@ # Defaults to -1. # #seccomp_sandbox = 1 + + + +# Override the listen address for all incoming migrations. Defaults to +# 0.0.0.0 or :: in case if both host and qemu are capable of IPv6. +#migration_address = "127.0.0.1" diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 1a41caf..16a0b92 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -546,6 +546,8 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, GET_VALUE_LONG("seccomp_sandbox", cfg->seccompSandbox); + GET_VALUE_STR("migration_address", cfg->migrationAddress); + ret = 0; cleanup: diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index da29a2a..40adfce 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -155,6 +155,9 @@ struct _virQEMUDriverConfig { unsigned int keepAliveCount; int seccompSandbox; + + /* The default for -incoming */ + char *migrationAddress; }; /* Main driver state */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e12a1de..7d87b2d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10264,6 +10264,7 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn, unsigned int flags) { virQEMUDriverPtr driver = dconn->privateData; + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); virDomainDefPtr def = NULL; const char *dom_xml = NULL; const char *dname = NULL; @@ -10306,6 +10307,11 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn, if (virDomainMigratePrepare3ParamsEnsureACL(dconn, def) < 0) goto cleanup; + /* If no listenAddress has been passed via @params, + * use the one from qemu.conf */ + if (!listenAddress) + listenAddress = cfg->migrationAddress; + ret = qemuMigrationPrepareDirect(driver, dconn, cookiein, cookieinlen, cookieout, cookieoutlen, @@ -10315,6 +10321,7 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn, cleanup: VIR_FREE(origname); virDomainDefFree(def); + virObjectUnref(cfg); return ret; } diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index ea770dc..4b27db1 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -66,3 +66,4 @@ module Test_libvirtd_qemu = { "keepalive_interval" = "5" } { "keepalive_count" = "5" } { "seccomp_sandbox" = "1" } +{ "migration_address" = "127.0.0.1" } -- 1.8.1.5

On 10/09/2013 03:32 PM, Michal Privoznik wrote:
This configuration knob is there to override default listen address for -incoming for all qemu domains.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/libvirtd_qemu.aug | 3 +++ src/qemu/qemu.conf | 6 ++++++ src/qemu/qemu_conf.c | 2 ++ src/qemu/qemu_conf.h | 3 +++ src/qemu/qemu_driver.c | 7 +++++++ src/qemu/test_libvirtd_qemu.aug.in | 1 + 6 files changed, 22 insertions(+)
diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index cd13d53..591d78d 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -78,6 +78,8 @@ module Libvirtd_qemu = | int_entry "keepalive_interval" | int_entry "keepalive_count"
+ let network_entry = str_entry "migration_address" + (* Each entry in the config is one of the following ... *) let entry = vnc_entry | spice_entry @@ -88,6 +90,7 @@ module Libvirtd_qemu = | process_entry | device_entry | rpc_entry + | network_entry
let comment = [ label "#comment" . del /#[ \t]*/ "# " . store /([^ \t\n][^\n]*)?/ . del /\n/ "\n" ] let empty = [ label "#empty" . eol ] diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 5fd6263..4403aaf 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -424,3 +424,9 @@ # Defaults to -1. # #seccomp_sandbox = 1 + + + +# Override the listen address for all incoming migrations. Defaults to +# 0.0.0.0 or :: in case if both host and qemu are capable of IPv6. +#migration_address = "127.0.0.1" diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 1a41caf..16a0b92 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -546,6 +546,8 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
GET_VALUE_LONG("seccomp_sandbox", cfg->seccompSandbox);
+ GET_VALUE_STR("migration_address", cfg->migrationAddress); + ret = 0;
cleanup: diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index da29a2a..40adfce 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -155,6 +155,9 @@ struct _virQEMUDriverConfig { unsigned int keepAliveCount;
int seccompSandbox; + + /* The default for -incoming */ + char *migrationAddress; };
/* Main driver state */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e12a1de..7d87b2d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10264,6 +10264,7 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn, unsigned int flags) { virQEMUDriverPtr driver = dconn->privateData; + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
You'll leak a reference to cfg if @flags or @params are invalid.
virDomainDefPtr def = NULL; const char *dom_xml = NULL; const char *dname = NULL; @@ -10306,6 +10307,11 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn, if (virDomainMigratePrepare3ParamsEnsureACL(dconn, def) < 0) goto cleanup;
+ /* If no listenAddress has been passed via @params, + * use the one from qemu.conf */ + if (!listenAddress) + listenAddress = cfg->migrationAddress; +
You can initialize listenAddress to this instead of NULL. ACK with the leak fixed.
participants (3)
-
Eric Blake
-
Ján Tomko
-
Michal Privoznik