[libvirt] [PATCH v4]: set and restore MAC address of a NIC when using PASSTHROUGH mode

[Laine Stump]

On 06/21/2011 03:25 PM, Eric Blake wrote:
> On 06/21/2011 12:21 PM, Stefan Berger wrote:
>> On 06/21/2011 01:16 PM, Eric Blake wrote:
>>> On 06/21/2011 06:01 AM, Gerhard Stenzel wrote:
>>>> +    rc = ifaceGetMacaddr(linkdev, oldmac);
>>>> +
>>>> +    if (rc) {
>>>> +        virReportSystemError(rc,
>>> Sorry for not catching this sooner.  This should probably be
>>> virReportSystemError(errno,...), assuming that ifaceGetMacaddr
>>> guarantees a sane errno setting (at any rate, you used errno for
>>> ifaceSetMacaddr later on).
>>>
>> It should be ok the way it is since ifaceGetMacAddr() returns either
>> EINVAL or the content of errno.
> Oh, I see.  But then we have the converse problem:
>
> +    rc = ifaceSetMacaddr(linkdev, macaddress);
> +    if (rc) {
> +        virReportSystemError(errno,
> +                             _("Setting MAC address on  '%s' to "
> +                               "'%02x:%02x:%02x:%02x:%02x:%02x' failed."),
>
> But ifaceSetMacaddr returns EINVAL without setting errno on at least one
> error path, which means this can print a bogus errno value.
>
> So we should either fix the iface* functions to follow convention of -1
> return and valid errno used in other libvirt interfaces (rather than
> positive return), as well as fix its clients; or we should fix this
> particular ifaceSetMacaddr call to fit in with the different convention.


I vote for consistency with the rest of the project, although I think 
that should be a separate patch (done as a prerequisite to this one)