[libvirt] [PATCH v2 0/4] Check migration configuration
add some check in migration configuration. Chen Fan (4): virsocketaddr: return address family in virSocketAddrIsNumeric migration: add migration_host support for Ipv6 address without brackets conf: add virSocketAddrIsLocalhost to Check migration_host conf: Check migration_address whether is localhost src/libvirt_private.syms | 1 + src/qemu/qemu.conf | 2 +- src/qemu/qemu_conf.c | 15 ++++++++++++ src/qemu/qemu_migration.c | 24 ++++++++++--------- src/qemu/test_libvirtd_qemu.aug.in | 2 +- src/util/virsocketaddr.c | 48 ++++++++++++++++++++++++++++++++++---- src/util/virsocketaddr.h | 5 +++- tests/sockettest.c | 2 +- 8 files changed, 80 insertions(+), 19 deletions(-) -- 1.9.3
nowadays, virSocketAddrIsNumeric only validated the income address if numeric, but sometimes we need to know whether the address is an IPv4 or an IPv6 address. Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com> --- src/qemu/qemu_migration.c | 4 ++-- src/util/virsocketaddr.c | 13 +++++++++---- src/util/virsocketaddr.h | 2 +- tests/sockettest.c | 2 +- 4 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index ce1a5cd..155f5b9 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2565,7 +2565,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, virObjectUnref(qemuCaps); if (listenAddress) { - if (virSocketAddrIsNumeric(listenAddress)) { + if (virSocketAddrIsNumeric(listenAddress, NULL)) { /* listenAddress is numeric IPv4 or IPv6 */ if (virSocketAddrParse(&listenAddressSocket, listenAddress, AF_UNSPEC) < 0) goto cleanup; @@ -2850,7 +2850,7 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver, goto cleanup; if (migrateHost != NULL) { - if (virSocketAddrIsNumeric(migrateHost) && + if (virSocketAddrIsNumeric(migrateHost, NULL) && virSocketAddrParse(NULL, migrateHost, AF_UNSPEC) < 0) goto cleanup; diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c index 7cc4bde..64409a6 100644 --- a/src/util/virsocketaddr.c +++ b/src/util/virsocketaddr.c @@ -858,6 +858,7 @@ virSocketAddrGetIpPrefix(const virSocketAddr *address, /** * virSocketAddrIsNumeric: * @address: address to check + * @family: where to store the address family, optional. * * Check if passed address is an IP address in numeric format. For * instance, for 0.0.0.0 true is returned, for 'examplehost" @@ -867,15 +868,19 @@ virSocketAddrGetIpPrefix(const virSocketAddr *address, * false otherwise */ bool -virSocketAddrIsNumeric(const char *address) +virSocketAddrIsNumeric(const char *address, int *family) { struct addrinfo *res; - unsigned short family; + unsigned short sa_family; if (virSocketAddrParseInternal(&res, address, AF_UNSPEC, false) < 0) return false; - family = res->ai_addr->sa_family; + sa_family = res->ai_addr->sa_family; freeaddrinfo(res); - return family == AF_INET || family == AF_INET6; + + if (family != NULL) { + *family = sa_family; + } + return sa_family == AF_INET || sa_family == AF_INET6; } diff --git a/src/util/virsocketaddr.h b/src/util/virsocketaddr.h index 27defa0..7b11afb 100644 --- a/src/util/virsocketaddr.h +++ b/src/util/virsocketaddr.h @@ -125,5 +125,5 @@ bool virSocketAddrIsPrivate(const virSocketAddr *addr); bool virSocketAddrIsWildcard(const virSocketAddr *addr); -bool virSocketAddrIsNumeric(const char *address); +bool virSocketAddrIsNumeric(const char *address, int *family); #endif /* __VIR_SOCKETADDR_H__ */ diff --git a/tests/sockettest.c b/tests/sockettest.c index 68b0536..dde0bb8 100644 --- a/tests/sockettest.c +++ b/tests/sockettest.c @@ -229,7 +229,7 @@ testIsNumericHelper(const void *opaque) { const struct testIsNumericData *data = opaque; - if (virSocketAddrIsNumeric(data->addr)) + if (virSocketAddrIsNumeric(data->addr, NULL)) return data->pass ? 0 : -1; return data->pass ? -1 : 0; } -- 1.9.3
if specifying migration_host to an Ipv6 address without brackets, it was resolved to an incorrect address, such as: tcp:2001:0DB8::1428:4444, but the correct address should be: tcp:[2001:0DB8::1428]:4444 so we should add brackets when parsing it. Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com> --- src/qemu/qemu_migration.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 155f5b9..016d131 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2544,7 +2544,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, if (VIR_STRDUP(migrateFrom, "stdio") < 0) goto cleanup; } else { - virSocketAddr listenAddressSocket; + int listenAddressFamily; bool encloseAddress = false; bool hostIPv6Capable = false; bool qemuIPv6Capable = false; @@ -2565,13 +2565,9 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, virObjectUnref(qemuCaps); if (listenAddress) { - if (virSocketAddrIsNumeric(listenAddress, NULL)) { + if (virSocketAddrIsNumeric(listenAddress, &listenAddressFamily)) { /* 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 (listenAddressFamily == AF_INET6) { if (!qemuIPv6Capable) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("qemu isn't capable of IPv6")); @@ -2850,11 +2846,17 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver, goto cleanup; if (migrateHost != NULL) { - if (virSocketAddrIsNumeric(migrateHost, NULL) && - virSocketAddrParse(NULL, migrateHost, AF_UNSPEC) < 0) - goto cleanup; + int family; + bool migrateHostisIpv6Address = false; + + if (virSocketAddrIsNumeric(migrateHost, &family) && + (family == AF_INET6)) + migrateHostisIpv6Address = true; - if (VIR_STRDUP(hostname, migrateHost) < 0) + if ((migrateHostisIpv6Address && + virAsprintf(&hostname, "[%s]", migrateHost) < 0) || + (!migrateHostisIpv6Address && + virAsprintf(&hostname, "%s", migrateHost) < 0)) goto cleanup; } else { if ((hostname = virGetHostname()) == NULL) -- 1.9.3
On 09/23/2014 06:04 AM, Chen Fan wrote:
if specifying migration_host to an Ipv6 address without brackets, it was resolved to an incorrect address, such as: tcp:2001:0DB8::1428:4444, but the correct address should be: tcp:[2001:0DB8::1428]:4444 so we should add brackets when parsing it.
Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com> --- src/qemu/qemu_migration.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 155f5b9..016d131 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2544,7 +2544,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, if (VIR_STRDUP(migrateFrom, "stdio") < 0) goto cleanup; } else { - virSocketAddr listenAddressSocket; + int listenAddressFamily; bool encloseAddress = false; bool hostIPv6Capable = false; bool qemuIPv6Capable = false; @@ -2565,13 +2565,9 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, virObjectUnref(qemuCaps);
if (listenAddress) { - if (virSocketAddrIsNumeric(listenAddress, NULL)) { + if (virSocketAddrIsNumeric(listenAddress, &listenAddressFamily)) {
Both uses of this function are in this patch (I didn't realize that when reviewing v1). We can rename it to virSocketAddrNumericFamily and do if (virSocketAddrNumericFamily(listenAddress) == AF_INET6), removing the need for a temporary variable.
/* 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 (listenAddressFamily == AF_INET6) { if (!qemuIPv6Capable) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("qemu isn't capable of IPv6")); @@ -2850,11 +2846,17 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver, goto cleanup;
if (migrateHost != NULL) { - if (virSocketAddrIsNumeric(migrateHost, NULL) && - virSocketAddrParse(NULL, migrateHost, AF_UNSPEC) < 0) - goto cleanup; + int family; + bool migrateHostisIpv6Address = false; + + if (virSocketAddrIsNumeric(migrateHost, &family) && + (family == AF_INET6)) + migrateHostisIpv6Address = true;
- if (VIR_STRDUP(hostname, migrateHost) < 0) + if ((migrateHostisIpv6Address && + virAsprintf(&hostname, "[%s]", migrateHost) < 0) || + (!migrateHostisIpv6Address && + virAsprintf(&hostname, "%s", migrateHost) < 0)) goto cleanup;
In qemuMigrationPrepareAny, we have: if (encloseAddress) incFormat = "%s:[%s]:%d"; else incFormat = "%s:%s:%d"; if (virAsprintf(&migrateFrom, incFormat, protocol, listenAddress, port) < 0) goto cleanup; Using the same pattern here as well would make it more readable. Jan
} else { if ((hostname = virGetHostname()) == NULL)
On Fri, 2014-10-03 at 15:58 +0200, Ján Tomko wrote:
On 09/23/2014 06:04 AM, Chen Fan wrote:
if specifying migration_host to an Ipv6 address without brackets, it was resolved to an incorrect address, such as: tcp:2001:0DB8::1428:4444, but the correct address should be: tcp:[2001:0DB8::1428]:4444 so we should add brackets when parsing it.
Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com> --- src/qemu/qemu_migration.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 155f5b9..016d131 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2544,7 +2544,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, if (VIR_STRDUP(migrateFrom, "stdio") < 0) goto cleanup; } else { - virSocketAddr listenAddressSocket; + int listenAddressFamily; bool encloseAddress = false; bool hostIPv6Capable = false; bool qemuIPv6Capable = false; @@ -2565,13 +2565,9 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, virObjectUnref(qemuCaps);
if (listenAddress) { - if (virSocketAddrIsNumeric(listenAddress, NULL)) { + if (virSocketAddrIsNumeric(listenAddress, &listenAddressFamily)) {
Both uses of this function are in this patch (I didn't realize that when reviewing v1).
We can rename it to virSocketAddrNumericFamily and do if (virSocketAddrNumericFamily(listenAddress) == AF_INET6), removing the need for a temporary variable. Right, Agreed.
/* 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 (listenAddressFamily == AF_INET6) { if (!qemuIPv6Capable) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("qemu isn't capable of IPv6")); @@ -2850,11 +2846,17 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver, goto cleanup;
if (migrateHost != NULL) { - if (virSocketAddrIsNumeric(migrateHost, NULL) && - virSocketAddrParse(NULL, migrateHost, AF_UNSPEC) < 0) - goto cleanup; + int family; + bool migrateHostisIpv6Address = false; + + if (virSocketAddrIsNumeric(migrateHost, &family) && + (family == AF_INET6)) + migrateHostisIpv6Address = true;
- if (VIR_STRDUP(hostname, migrateHost) < 0) + if ((migrateHostisIpv6Address && + virAsprintf(&hostname, "[%s]", migrateHost) < 0) || + (!migrateHostisIpv6Address && + virAsprintf(&hostname, "%s", migrateHost) < 0)) goto cleanup;
In qemuMigrationPrepareAny, we have: if (encloseAddress) incFormat = "%s:[%s]:%d"; else incFormat = "%s:%s:%d"; if (virAsprintf(&migrateFrom, incFormat, protocol, listenAddress, port) < 0) goto cleanup;
Using the same pattern here as well would make it more readable.
Thanks, Chen
Jan
} else { if ((hostname = virGetHostname()) == NULL)
Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com> --- src/libvirt_private.syms | 1 + src/qemu/qemu_conf.c | 8 ++++++++ src/util/virsocketaddr.c | 35 +++++++++++++++++++++++++++++++++++ src/util/virsocketaddr.h | 3 +++ 4 files changed, 47 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 51a692b..f7172b0 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1885,6 +1885,7 @@ virSocketAddrGetPort; virSocketAddrGetRange; virSocketAddrIsNetmask; virSocketAddrIsNumeric; +virSocketAddrIsLocalhost; virSocketAddrIsPrivate; virSocketAddrIsWildcard; virSocketAddrMask; diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index adc6caf..30169cf 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -707,6 +707,14 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, GET_VALUE_LONG("seccomp_sandbox", cfg->seccompSandbox); GET_VALUE_STR("migration_host", cfg->migrateHost); + if (cfg->migrateHost && + virSocketAddrIsLocalhost(cfg->migrateHost)) { + virReportError(VIR_ERR_CONF_SYNTAX, + _("migration_host must not be 'localhost' address: %s"), + cfg->migrateHost); + goto cleanup; + } + GET_VALUE_STR("migration_address", cfg->migrationAddress); GET_VALUE_BOOL("log_timestamp", cfg->logTimestamp); diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c index 64409a6..dfcaf72 100644 --- a/src/util/virsocketaddr.c +++ b/src/util/virsocketaddr.c @@ -884,3 +884,38 @@ virSocketAddrIsNumeric(const char *address, int *family) } return sa_family == AF_INET || sa_family == AF_INET6; } + +/** + * virSocketAddrIsLocalhost: + * @address: address to check + * + * Check if passed address is a 'localhost' address. + * + * Returns: true if @address is 'localhost' address, + * false otherwise + */ +bool +virSocketAddrIsLocalhost(const char *address) +{ + int family; + + if (virSocketAddrIsNumeric(address, &family)) { + if (family == AF_INET) { + if (STREQ(address, "127.0.0.1")) + return true; + } + + if (family == AF_INET6) { + if (STREQ(address, "::1")) + return true; + } + } else { + if (STRPREFIX(address, "localhost")) + return true; + + if (STREQ(address, "[::1]")) + return true; + } + + return false; +} diff --git a/src/util/virsocketaddr.h b/src/util/virsocketaddr.h index 7b11afb..5269f35 100644 --- a/src/util/virsocketaddr.h +++ b/src/util/virsocketaddr.h @@ -126,4 +126,7 @@ bool virSocketAddrIsPrivate(const virSocketAddr *addr); bool virSocketAddrIsWildcard(const virSocketAddr *addr); bool virSocketAddrIsNumeric(const char *address, int *family); + +bool virSocketAddrIsLocalhost(const char *address); + #endif /* __VIR_SOCKETADDR_H__ */ -- 1.9.3
On 09/23/2014 06:04 AM, Chen Fan wrote:
Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com> --- src/libvirt_private.syms | 1 + src/qemu/qemu_conf.c | 8 ++++++++ src/util/virsocketaddr.c | 35 +++++++++++++++++++++++++++++++++++ src/util/virsocketaddr.h | 3 +++ 4 files changed, 47 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 51a692b..f7172b0 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1885,6 +1885,7 @@ virSocketAddrGetPort; virSocketAddrGetRange; virSocketAddrIsNetmask; virSocketAddrIsNumeric; +virSocketAddrIsLocalhost; virSocketAddrIsPrivate; virSocketAddrIsWildcard; virSocketAddrMask; diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index adc6caf..30169cf 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -707,6 +707,14 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, GET_VALUE_LONG("seccomp_sandbox", cfg->seccompSandbox);
GET_VALUE_STR("migration_host", cfg->migrateHost); + if (cfg->migrateHost && + virSocketAddrIsLocalhost(cfg->migrateHost)) { + virReportError(VIR_ERR_CONF_SYNTAX, + _("migration_host must not be 'localhost' address: %s"), + cfg->migrateHost); + goto cleanup; + } + GET_VALUE_STR("migration_address", cfg->migrationAddress);
GET_VALUE_BOOL("log_timestamp", cfg->logTimestamp); diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c index 64409a6..dfcaf72 100644 --- a/src/util/virsocketaddr.c +++ b/src/util/virsocketaddr.c @@ -884,3 +884,38 @@ virSocketAddrIsNumeric(const char *address, int *family) } return sa_family == AF_INET || sa_family == AF_INET6; } + +/** + * virSocketAddrIsLocalhost: + * @address: address to check + * + * Check if passed address is a 'localhost' address. + * + * Returns: true if @address is 'localhost' address, + * false otherwise + */ +bool +virSocketAddrIsLocalhost(const char *address)
I think this function should be named 'IsNumericLocalhost' and only check for the numeric representation of localhost. If the address is numeric, we can parse it and catch all the cases (like 127.0.0.1, 2130706433, 0177.0.0.1, 0:0:0::1). But we can't check if a hostname points to localhost without resolving it.
+{ + int family; + + if (virSocketAddrIsNumeric(address, &family)) { + if (family == AF_INET) { + if (STREQ(address, "127.0.0.1")) + return true; + } +
This should do what virSocketAddrIsWildcard does, only using INADDR_LOOPBACK instead of INADDR_ANY and IN6_IS_ADDR_LOOPBACK instead of IN6_IS_ADDR_UNSPECIFIED.
+ if (family == AF_INET6) { + if (STREQ(address, "::1")) + return true; + } + } else { + if (STRPREFIX(address, "localhost")) + return true;
I'd put this check in qemu_conf.c.
+ + if (STREQ(address, "[::1]")) + return true;
And strip the brackets before calling virSocketAddrParse. Jan
+ } + + return false; +} diff --git a/src/util/virsocketaddr.h b/src/util/virsocketaddr.h index 7b11afb..5269f35 100644 --- a/src/util/virsocketaddr.h +++ b/src/util/virsocketaddr.h @@ -126,4 +126,7 @@ bool virSocketAddrIsPrivate(const virSocketAddr *addr); bool virSocketAddrIsWildcard(const virSocketAddr *addr);
bool virSocketAddrIsNumeric(const char *address, int *family); + +bool virSocketAddrIsLocalhost(const char *address); + #endif /* __VIR_SOCKETADDR_H__ */
On Fri, 2014-10-03 at 15:58 +0200, Ján Tomko wrote:
On 09/23/2014 06:04 AM, Chen Fan wrote:
Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com> --- src/libvirt_private.syms | 1 + src/qemu/qemu_conf.c | 8 ++++++++ src/util/virsocketaddr.c | 35 +++++++++++++++++++++++++++++++++++ src/util/virsocketaddr.h | 3 +++ 4 files changed, 47 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 51a692b..f7172b0 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1885,6 +1885,7 @@ virSocketAddrGetPort; virSocketAddrGetRange; virSocketAddrIsNetmask; virSocketAddrIsNumeric; +virSocketAddrIsLocalhost; virSocketAddrIsPrivate; virSocketAddrIsWildcard; virSocketAddrMask; diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index adc6caf..30169cf 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -707,6 +707,14 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, GET_VALUE_LONG("seccomp_sandbox", cfg->seccompSandbox);
GET_VALUE_STR("migration_host", cfg->migrateHost); + if (cfg->migrateHost && + virSocketAddrIsLocalhost(cfg->migrateHost)) { + virReportError(VIR_ERR_CONF_SYNTAX, + _("migration_host must not be 'localhost' address: %s"), + cfg->migrateHost); + goto cleanup; + } + GET_VALUE_STR("migration_address", cfg->migrationAddress);
GET_VALUE_BOOL("log_timestamp", cfg->logTimestamp); diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c index 64409a6..dfcaf72 100644 --- a/src/util/virsocketaddr.c +++ b/src/util/virsocketaddr.c @@ -884,3 +884,38 @@ virSocketAddrIsNumeric(const char *address, int *family) } return sa_family == AF_INET || sa_family == AF_INET6; } + +/** + * virSocketAddrIsLocalhost: + * @address: address to check + * + * Check if passed address is a 'localhost' address. + * + * Returns: true if @address is 'localhost' address, + * false otherwise + */ +bool +virSocketAddrIsLocalhost(const char *address)
I think this function should be named 'IsNumericLocalhost' and only check for the numeric representation of localhost. If the address is numeric, we can parse it and catch all the cases (like 127.0.0.1, 2130706433, 0177.0.0.1, 0:0:0::1). But we can't check if a hostname points to localhost without resolving it.
+{ + int family; + + if (virSocketAddrIsNumeric(address, &family)) { + if (family == AF_INET) { + if (STREQ(address, "127.0.0.1")) + return true; + } +
This should do what virSocketAddrIsWildcard does, only using INADDR_LOOPBACK instead of INADDR_ANY and IN6_IS_ADDR_LOOPBACK instead of IN6_IS_ADDR_UNSPECIFIED.
+ if (family == AF_INET6) { + if (STREQ(address, "::1")) + return true; + } + } else { + if (STRPREFIX(address, "localhost")) + return true;
I'd put this check in qemu_conf.c.
+ + if (STREQ(address, "[::1]")) + return true;
And strip the brackets before calling virSocketAddrParse.
I had sent V3 patch that including the above solution. please help to review it. Thanks, Chen
Jan
+ } + + return false; +} diff --git a/src/util/virsocketaddr.h b/src/util/virsocketaddr.h index 7b11afb..5269f35 100644 --- a/src/util/virsocketaddr.h +++ b/src/util/virsocketaddr.h @@ -126,4 +126,7 @@ bool virSocketAddrIsPrivate(const virSocketAddr *addr); bool virSocketAddrIsWildcard(const virSocketAddr *addr);
bool virSocketAddrIsNumeric(const char *address, int *family); + +bool virSocketAddrIsLocalhost(const char *address); + #endif /* __VIR_SOCKETADDR_H__ */
When enabling the migration_address option, by default it is set to "127.0.0.1", but it's not a valid address for migration. so we should add verification and set the default migration_address to "0.0.0.0". Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com> --- src/qemu/qemu.conf | 2 +- src/qemu/qemu_conf.c | 7 +++++++ src/qemu/test_libvirtd_qemu.aug.in | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 79bba36..666c303 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -459,7 +459,7 @@ # Override the listen address for all incoming migrations. Defaults to # 0.0.0.0, or :: if both host and qemu are capable of IPv6. -#migration_address = "127.0.0.1" +#migration_address = "0.0.0.0" # The default hostname or IP address which will be used by a migration diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 30169cf..65f98d7 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -716,6 +716,13 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, } GET_VALUE_STR("migration_address", cfg->migrationAddress); + if (cfg->migrationAddress && + virSocketAddrIsLocalhost(cfg->migrationAddress)) { + virReportError(VIR_ERR_CONF_SYNTAX, + _("migration_address must not be 'localhost' address: %s"), + cfg->migrationAddress); + goto cleanup; + } GET_VALUE_BOOL("log_timestamp", cfg->logTimestamp); diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index d2bc2c0..30fd27e 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -69,7 +69,7 @@ module Test_libvirtd_qemu = { "keepalive_interval" = "5" } { "keepalive_count" = "5" } { "seccomp_sandbox" = "1" } -{ "migration_address" = "127.0.0.1" } +{ "migration_address" = "0.0.0.0" } { "migration_host" = "host.example.com" } { "migration_port_min" = "49152" } { "migration_port_max" = "49215" } -- 1.9.3
ping? On Tue, 2014-09-23 at 12:04 +0800, Chen Fan wrote:
add some check in migration configuration.
Chen Fan (4): virsocketaddr: return address family in virSocketAddrIsNumeric migration: add migration_host support for Ipv6 address without brackets conf: add virSocketAddrIsLocalhost to Check migration_host conf: Check migration_address whether is localhost
src/libvirt_private.syms | 1 + src/qemu/qemu.conf | 2 +- src/qemu/qemu_conf.c | 15 ++++++++++++ src/qemu/qemu_migration.c | 24 ++++++++++--------- src/qemu/test_libvirtd_qemu.aug.in | 2 +- src/util/virsocketaddr.c | 48 ++++++++++++++++++++++++++++++++++---- src/util/virsocketaddr.h | 5 +++- tests/sockettest.c | 2 +- 8 files changed, 80 insertions(+), 19 deletions(-)
any feedback? Thanks, Chen On Thu, 2014-09-25 at 01:10 +0000, Chen, Fan wrote:
ping?
On Tue, 2014-09-23 at 12:04 +0800, Chen Fan wrote:
add some check in migration configuration.
Chen Fan (4): virsocketaddr: return address family in virSocketAddrIsNumeric migration: add migration_host support for Ipv6 address without brackets conf: add virSocketAddrIsLocalhost to Check migration_host conf: Check migration_address whether is localhost
src/libvirt_private.syms | 1 + src/qemu/qemu.conf | 2 +- src/qemu/qemu_conf.c | 15 ++++++++++++ src/qemu/qemu_migration.c | 24 ++++++++++--------- src/qemu/test_libvirtd_qemu.aug.in | 2 +- src/util/virsocketaddr.c | 48 ++++++++++++++++++++++++++++++++++---- src/util/virsocketaddr.h | 5 +++- tests/sockettest.c | 2 +- 8 files changed, 80 insertions(+), 19 deletions(-)
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (3)
-
Chen Fan -
Chen, Fan
-
Ján Tomko