[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCHv2 07/13] Replace brSetInetAddress/brSetInetNetmask with brAddInetAddress



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

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]