On 12/22/2010 11:58 AM, Laine Stump wrote:
brSetInetAddress can only set a single IP address on the bridge, and uses a method (ioctl(SIOCSETIFADDR)) that only works for IPv4. Replace it and brSetInetNetmask with a single function that uses the external "ip addr add" command to add an address/prefix to the interface - this supports IPv6, and allows adding multiple addresses to the interface.
Although it isn't currently used in the code, we also add a brDelInetAddress for completeness' sake.
Also, while we're modifying bridge.c, we change brSetForwardDelay and brSetEnableSTP to use the new virCommand API rather than the deprecated virRun, and also log an error message in bridge_driver.c if either of those fail (previously the failure would be completely silent).
NB: it's a bit bothersome that there is no difference in error reporting between being unable to run one of these commands, and the command returning a non-0 exit status, but there is no precedent in bridge.c for logging error messages locally rather than pushing them up the call chain, and what's pushed up is only 'success' or 'failure'; no exit codes. Perhaps a non-0 exit could be passed up directly as the return, other errors could return -1, and callers could check for ret != 0 rather than ret < 0?
Food for thought, but doesn't impact the quality of this patch.
+ if (VIR_SOCKET_HAS_ADDR(&network->def->ipAddress)) { + int prefix = virNetworkDefPrefix(network->def); + + if (prefix < 0) { + networkReportError(VIR_ERR_INTERNAL_ERROR, + _("bridge '%s' has an invalid netmask or IP address"),
As long as you're touching this, collapse the two spaces in the message to one. ACK with that nit fixed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org