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)
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list