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.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library
http://libvirt.org