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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list