On 02/25/2013 07:15 AM, Ján Tomko wrote: > Use virURIParse in qemuMigrationPrepareDirect to allow parsing > IPv6 addresses, which would cause an 'incorrect :port' error message > before. > > To be able to migrate over IPv6, QEMU needs to listen on [::] instead > of 0.0.0.0. This patch adds a call to getaddrinfo and sets the listen > address based on the result. > > It also uses the same listen address for the NBD server. > > This will break migration if a hostname does not resolve to the same > address family on both sides. > > Bug: https://bugzilla.redhat.com/show_bug.cgi?id=846013 > --- > > Diff to V1: > * initialize uri_str > * reuse STRSKIP("tcp:") result instead of doing strlen on it > * print a warning instead of failing when the hostname can't be resolved > > Diff to V2: > * freeaddrinfo > * separate the listen address to allow reuse in qemuMigrationStartNBDServer > > src/qemu/qemu_migration.c | 77 ++++++++++++++++++++++++++++++++++++----------- > 1 file changed, 59 insertions(+), 18 deletions(-) > > diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c > index cae58fa..ff9b959 100644 > --- a/src/qemu/qemu_migration.c > +++ b/src/qemu/qemu_migration.c > @@ -22,7 +22,10 @@ > > #include <config.h> > > +#include <netdb.h> > +#include <sys/socket.h> > #include <sys/time.h> > +#include <sys/types.h> You rarely ever need <sys/types.h> - most types in that header are guaranteed to be provided by any other standard header that has an interface using that type; or put another way, POSIX says that inclusion of other headers may drag in <sys/types.h>, and we have already been relying on that reliably happening. The other two additions are important, though. > > @@ -2292,9 +2296,14 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver, > static int port = 0; > int this_port; > char *hostname = NULL; > + char listenAddr[8]; > char migrateFrom [64]; As long as you are touching this, remove the space before [64]. Then again, 64 was a magic number; a much safer approach would have been char migrateFrom[sizeof("tcp:0.0.0.0:") + INT_BUFSIZE_BOUND(int)]; (using gnulib's "intprops.h"), or even getting rid of the stack-alloc temporary (see below for thoughts about using virBufferPtr instead) > > + if (getaddrinfo(hostname, NULL, NULL, &info)) { > + VIR_WARN("unable to get address info for %s, defaulting to IPv4", > + hostname); Would it be nice to include the gai_strerror() reason for why getaddrinfo() failed? > + } else { > + ipv6 = info->ai_family == AF_INET6; > + freeaddrinfo(info); > + } > + > if (*uri_out) > VIR_DEBUG("Generated uri_out=%s", *uri_out); > > - /* QEMU will be started with -incoming tcp:0.0.0.0:port */ > - snprintf(migrateFrom, sizeof(migrateFrom), "tcp:0.0.0.0:%d", this_port); > + /* QEMU will be started with -incoming tcp:0.0.0.0:port > + * or -incoming tcp:[::]:port for IPv6 */ > + if (ipv6) > + snprintf(listenAddr, sizeof(listenAddr), "[::]"); > + else > + snprintf(listenAddr, sizeof(listenAddr), "0.0.0.0"); Eww. Even though this is safe (because you sized listenAddr big enough), I'd rather see: const char *listenAddr; if (ipv6) listenAddr = "[::]"; else listenAddr = "0.0.0.0"; > + > + snprintf(migrateFrom, sizeof(migrateFrom), > + "tcp:%s:%d", listenAddr, this_port); And while this snprintf was pre-existing and also happens to be large enough, I can't help but think it would be nicer to pass a virBufferPtr instead of a const char * to qemuMigrationPrepareAny, although that would mean more refactoring done in an independent patch. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list