[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]