[libvirt] [PATCH 0/2] Support IPv6 in port allocator

Ján Tomko (2): Split out virHostHasIPv6 from qemuMigrationPrepareAny Bind to both IPv4 and v6 when checking port availability src/libvirt_private.syms | 1 + src/qemu/qemu_migration.c | 8 +------- src/util/virportallocator.c | 34 +++++++++++++++++++++++++++------- src/util/virutil.c | 14 ++++++++++++++ src/util/virutil.h | 1 + 5 files changed, 44 insertions(+), 14 deletions(-) -- 1.8.1.5

Split out the part of qemuMigrationPrepareAny that decides whether to listen on [::] or 0.0.0.0 by checking whether IPv6 is enabled on the host. --- src/libvirt_private.syms | 1 + src/qemu/qemu_migration.c | 8 +------- src/util/virutil.c | 14 ++++++++++++++ src/util/virutil.h | 1 + 4 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 4bc4d69..157d68c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2065,6 +2065,7 @@ virGetUserID; virGetUserName; virGetUserRuntimeDirectory; virHexToBin; +virHostHasIPv6; virIndexToDiskName; virIsCapableFCHost; virIsCapableVport; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 3a1aab7..c21063d 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -22,8 +22,6 @@ #include <config.h> -#include <netdb.h> -#include <sys/socket.h> #include <sys/time.h> #ifdef WITH_GNUTLS # include <gnutls/gnutls.h> @@ -2261,9 +2259,6 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, goto cleanup; } else { virQEMUCapsPtr qemuCaps = NULL; - struct addrinfo *info = NULL; - struct addrinfo hints = { .ai_flags = AI_ADDRCONFIG, - .ai_socktype = SOCK_STREAM }; if (!(qemuCaps = virQEMUCapsCacheLookupCopy(driver->qemuCapsCache, (*def)->emulator))) @@ -2273,8 +2268,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, * and there is at least one IPv6 address configured */ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_IPV6_MIGRATION) && - getaddrinfo("::", NULL, &hints, &info) == 0) { - freeaddrinfo(info); + virHostHasIPv6()) { listenAddr = "[::]"; } else { listenAddr = "0.0.0.0"; diff --git a/src/util/virutil.c b/src/util/virutil.c index d9e0bc4..2fcd8a5 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -648,6 +648,20 @@ cleanup: } +bool +virHostHasIPv6() +{ + bool ret; + struct addrinfo *info = NULL; + struct addrinfo hints = { .ai_flags = AI_ADDRCONFIG, + .ai_socktype = SOCK_STREAM }; + + ret = getaddrinfo("::", NULL, &hints, &info) == 0; + freeaddrinfo(info); + return ret; +} + + char * virGetUserDirectory(void) { diff --git a/src/util/virutil.h b/src/util/virutil.h index 4b06992..437e72c 100644 --- a/src/util/virutil.h +++ b/src/util/virutil.h @@ -109,6 +109,7 @@ static inline int getgid (void) { return 0; } # endif char *virGetHostname(void); +bool virHostHasIPv6(void); char *virGetUserDirectory(void); char *virGetUserDirectoryByUID(uid_t uid); -- 1.8.1.5

On Tue, Oct 01, 2013 at 11:01:36AM +0200, Ján Tomko wrote:
Split out the part of qemuMigrationPrepareAny that decides whether to listen on [::] or 0.0.0.0 by checking whether IPv6 is enabled on the host. --- src/libvirt_private.syms | 1 + src/qemu/qemu_migration.c | 8 +------- src/util/virutil.c | 14 ++++++++++++++ src/util/virutil.h | 1 + 4 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 4bc4d69..157d68c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2065,6 +2065,7 @@ virGetUserID; virGetUserName; virGetUserRuntimeDirectory; virHexToBin; +virHostHasIPv6; virIndexToDiskName; virIsCapableFCHost; virIsCapableVport; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 3a1aab7..c21063d 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -22,8 +22,6 @@
#include <config.h>
-#include <netdb.h> -#include <sys/socket.h> #include <sys/time.h> #ifdef WITH_GNUTLS # include <gnutls/gnutls.h> @@ -2261,9 +2259,6 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, goto cleanup; } else { virQEMUCapsPtr qemuCaps = NULL; - struct addrinfo *info = NULL; - struct addrinfo hints = { .ai_flags = AI_ADDRCONFIG, - .ai_socktype = SOCK_STREAM };
if (!(qemuCaps = virQEMUCapsCacheLookupCopy(driver->qemuCapsCache, (*def)->emulator))) @@ -2273,8 +2268,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, * and there is at least one IPv6 address configured */ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_IPV6_MIGRATION) && - getaddrinfo("::", NULL, &hints, &info) == 0) { - freeaddrinfo(info); + virHostHasIPv6()) { listenAddr = "[::]"; } else { listenAddr = "0.0.0.0"; diff --git a/src/util/virutil.c b/src/util/virutil.c index d9e0bc4..2fcd8a5 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -648,6 +648,20 @@ cleanup: }
+bool +virHostHasIPv6() +{ + bool ret; + struct addrinfo *info = NULL; + struct addrinfo hints = { .ai_flags = AI_ADDRCONFIG, + .ai_socktype = SOCK_STREAM }; + + ret = getaddrinfo("::", NULL, &hints, &info) == 0; + freeaddrinfo(info); + return ret; +}
This really doesn't belong in virutil.c. I'd prefer to see this in virnetdev.{c,h} and named virNetDevCheckIPProtocols(bool *hasIPv4, bool *hasIPv6) and it should use 'getifaddrs' - 'getaddrinfo' isn't a reliable way to determine if IPv6 is present. There is code in tests/virnetsockettest.c you can move for this purpose. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

If IPv6 is enabled on the host, bind to :: with IPV6_V6ONLY disabled to check for ports available on both IPv6 and IPv4. --- src/util/virportallocator.c | 34 +++++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c index 5b7ad41..246f6cd 100644 --- a/src/util/virportallocator.c +++ b/src/util/virportallocator.c @@ -99,14 +99,25 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa, int ret = -1; size_t i; int fd = -1; + bool ipv6 = virHostHasIPv6(); *port = 0; virObjectLock(pa); for (i = pa->start; i <= pa->end && !*port; i++) { - int reuse = 1; - struct sockaddr_in addr; + int reuse = 1, v6only = 0; + struct sockaddr_in addr = { + .sin_family = AF_INET, + .sin_port = htons(i), + .sin_addr.s_addr = htonl(INADDR_ANY) + }; + struct sockaddr_in6 addr6 = { + .sin6_family = AF_INET6, + .sin6_port = htons(i), + .sin6_addr = IN6ADDR_ANY_INIT + }; bool used = false; + int rc; if (virBitmapGetBit(pa->bitmap, i - pa->start, &used) < 0) { @@ -118,10 +129,7 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa, if (used) continue; - addr.sin_family = AF_INET; - addr.sin_port = htons(i); - addr.sin_addr.s_addr = htonl(INADDR_ANY); - fd = socket(PF_INET, SOCK_STREAM, 0); + fd = socket(ipv6 ? PF_INET6 : PF_INET, SOCK_STREAM, 0); if (fd < 0) { virReportSystemError(errno, "%s", _("Unable to open test socket")); @@ -134,7 +142,19 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa, goto cleanup; } - if (bind(fd, (struct sockaddr*)&addr, sizeof(addr)) < 0) { + if (ipv6 && setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, (void*)&v6only, + sizeof(v6only)) < 0) { + virReportSystemError(errno, "%s", + _("Unable to unset socket IPV6_V6ONLY flag")); + goto cleanup; + } + + if (ipv6) + rc = bind(fd, (struct sockaddr*)&addr6, sizeof(addr6)); + else + rc = bind(fd, (struct sockaddr*)&addr, sizeof(addr)); + + if (rc < 0) { if (errno != EADDRINUSE) { virReportSystemError(errno, _("Unable to bind to port %zu"), i); -- 1.8.1.5

On Tue, Oct 01, 2013 at 11:01:37AM +0200, Ján Tomko wrote:
If IPv6 is enabled on the host, bind to :: with IPV6_V6ONLY disabled to check for ports available on both IPv6 and IPv4. --- src/util/virportallocator.c | 34 +++++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-)
diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c index 5b7ad41..246f6cd 100644 --- a/src/util/virportallocator.c +++ b/src/util/virportallocator.c @@ -99,14 +99,25 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa, int ret = -1; size_t i; int fd = -1; + bool ipv6 = virHostHasIPv6();
*port = 0; virObjectLock(pa);
for (i = pa->start; i <= pa->end && !*port; i++) { - int reuse = 1; - struct sockaddr_in addr; + int reuse = 1, v6only = 0; + struct sockaddr_in addr = { + .sin_family = AF_INET, + .sin_port = htons(i), + .sin_addr.s_addr = htonl(INADDR_ANY) + }; + struct sockaddr_in6 addr6 = { + .sin6_family = AF_INET6, + .sin6_port = htons(i), + .sin6_addr = IN6ADDR_ANY_INIT + }; bool used = false; + int rc;
if (virBitmapGetBit(pa->bitmap, i - pa->start, &used) < 0) { @@ -118,10 +129,7 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa, if (used) continue;
- addr.sin_family = AF_INET; - addr.sin_port = htons(i); - addr.sin_addr.s_addr = htonl(INADDR_ANY); - fd = socket(PF_INET, SOCK_STREAM, 0); + fd = socket(ipv6 ? PF_INET6 : PF_INET, SOCK_STREAM, 0); if (fd < 0) { virReportSystemError(errno, "%s", _("Unable to open test socket")); @@ -134,7 +142,19 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa, goto cleanup; }
- if (bind(fd, (struct sockaddr*)&addr, sizeof(addr)) < 0) { + if (ipv6 && setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, (void*)&v6only, + sizeof(v6only)) < 0) { + virReportSystemError(errno, "%s", + _("Unable to unset socket IPV6_V6ONLY flag")); + goto cleanup; + } + + if (ipv6) + rc = bind(fd, (struct sockaddr*)&addr6, sizeof(addr6)); + else + rc = bind(fd, (struct sockaddr*)&addr, sizeof(addr));
IMHO you could write this code without needing to have any upfront check for whether IPv6 is enabled. Set IPV6_V6ONLY == 1. Then unconditionally try to bind separately on IPv4, and IPv6, and catch the error you'd get back if either IPv4 or IPv6 were disabled on the host. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (2)
-
Daniel P. Berrange
-
Ján Tomko