On 05/27/2010 04:54 AM, Jim Meyering wrote:
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
v3: Consider route netmask. Compare binary data rather than convert to string. ... +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[32]; + + 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; + if (!inet_ntop(AF_INET, &inaddress, netaddr, sizeof(netaddr))) { + virReportSystemError(errno, "%s", + _("failed to format network address")); + goto error; + } + + /* Read whole routing table into memory */ + if ((len = virFileReadAll(PROC_NET_ROUTE, MAX_ROUTE_SIZE, &buf)) < 0) + goto error;
+ /* Dropping the last character shouldn't hurt */
Hi Cole,
Not sure how much you want to invest in manual parsing, but in case this code ends up staying with us for a while, here are some suggestions.
Handle the case in which the file is empty. This will appease static checkers like clang and coverity.
Either change the "< 0" test above to "<= 0" or do e.g.,
if (len > 0)
+ buf[len-1] = '\0';
Future proof it by skipping that first line only if it looks like the heading we see now:
if (STRPREFIX (buf, "Iface"))
Thanks for the review, I incorporated the above recommendations. However, I went back to using sscanf which greatly simplifies the parsing, and is used in a safe manner AFAICT. Updated patch coming shortly. Thanks, Cole