On Wed, May 26, 2010 at 03:56:17PM -0400, Cole Robinson wrote: > On 05/26/2010 02:05 PM, Laine Stump wrote: > > A couple items I didn't notice before: > > > > On 05/24/2010 02:52 PM, Cole Robinson wrote: > >> Fedora bug https://bugzilla.redhat.com/show_bug.cgi?id=235961 > >> > >> If using the default virtual network, an easy way to lose guest network > >> connectivity is to install libvirt inside the VM. The autostarted > >> default network inside the guest collides with host virtual network > >> routing. This is a long standing issue that has caused users quite a > >> bit of pain and confusion. > >> > >> On network startup, parse /proc/net/route and compare the requested > >> IP+netmask against host routing destinations: if any matches are found, > >> refuse to start the network. > >> > >> v2: Drop sscanf, fix a comment typo, comment that function could use > >> libnl instead of /proc > >> > >> Signed-off-by: Cole Robinson<crobinso@xxxxxxxxxx> > >> --- > >> src/network/bridge_driver.c | 102 +++++++++++++++++++++++++++++++++++++++++++ > >> 1 files changed, 102 insertions(+), 0 deletions(-) > >> > >> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c > >> index 5d7ef19..090bed7 100644 > >> --- a/src/network/bridge_driver.c > >> +++ b/src/network/bridge_driver.c > >> @@ -42,6 +42,8 @@ > >> #include<stdio.h> > >> #include<sys/wait.h> > >> #include<sys/ioctl.h> > >> +#include<netinet/in.h> > >> +#include<arpa/inet.h> > >> > >> #include "virterror_internal.h" > >> #include "datatypes.h" > >> @@ -908,6 +910,102 @@ cleanup: > >> return ret; > >> } > >> > >> +#define PROC_NET_ROUTE "/proc/net/route" > >> + > >> +/* XXX: This function can be a lot more exhaustive, there are certainly > >> + * other scenarios where we can ruin host network connectivity. > >> + * XXX: Using a proper library is preferred over parsing /proc > >> + */ > >> +static int networkCheckRouteCollision(virNetworkObjPtr network) > >> +{ > >> + int ret = -1, len; > >> + char *cur, *buf = NULL; > >> + enum {MAX_ROUTE_SIZE = 1024*64}; > >> + struct in_addr inaddress, innetmask; > >> + char *netaddr = NULL; > >> + > >> + if (!network->def->ipAddress || !network->def->netmask) > >> + return 0; > >> + > >> + if (inet_pton(AF_INET, network->def->ipAddress,&inaddress)<= 0) { > >> + networkReportError(VIR_ERR_INTERNAL_ERROR, > >> + _("cannot parse IP address '%s'"), > >> + network->def->ipAddress); > >> + goto error; > >> + } > >> + if (inet_pton(AF_INET, network->def->netmask,&innetmask)<= 0) { > >> + networkReportError(VIR_ERR_INTERNAL_ERROR, > >> + _("cannot parse netmask '%s'"), > >> + network->def->netmask); > >> + goto error; > >> + } > >> + > >> + inaddress.s_addr&= innetmask.s_addr; > >> + netaddr = strdup(inet_ntoa(inaddress)); > >> > > > > inet_ntoa() isn't threadsafe (it re-uses the same static buffer). strdup > > reduces the window, but the potential for a bad result is still there. > > Better to use inet_ntop(), or possibly use virSocketFormatAddr() > > (although that would have a bit of setup involved). > > > > Will do. None of the inet_* functions should be used as they are all obsolete. Use one of the virSocket* functions in src/util/network.c - if there isn't a suitable one add a new one, based on getnameinfo() + AI_NUMERIC flag. Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list