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@redhat.com> --- 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 :|