[libvirt] [PATCH] Revert "utils: Remove the logging of errors from virNetDevSendEthtoolIoctl"

[John Ferlan]

On 11/03/2015 11:15 AM, Daniel P. Berrange wrote:

[...]

>> still still be spewed? Since virNetDevSetupControlFull:148 is the socket
>> call failure and not the ioctl failure. Sure the caller wouldn't spew a
>> second one, but why would the first one not be spewed?
> 
> Empirically I don't see any errors after reverting it. It appears the
> reason for this is that the original code uses AF_LOCAL while the
> virNetDevSetupControllFull uses AF_PACKET, and the latter is restricted
> to only privileged users. If I change the original code to use AF_PACKET
> then it shows errors too. The reason for using AF_PACKET was that it
> was substaintially faster than AF_LOCAL
> 

Alternatively in order to not fall into the rabbit hole again...

Can we determine if we're not privileged at the start of
virNetDevSendEthtoolIoctl, eg

   if (geteuid() != 0) {
        VIR_{WARN|INFO}("cannot set feature in unprivileged daemon");
        cmd->data = 0;  /* To indicate not found (e.g. lie) */
        return 0;
   }
or follow virNetDevBandwidthSet which causes an error.

IOW: Should we allow these bits to be set in a non privileged daemon?
And if not should we error on them?


For the concern about the two error messages, just remove the
virReportSystemError from the callers and adjust the ioctl() to:

if ((ret = ioctl(fd, SIOCETHTOOL, &ifr)) < 0) {
    virReportSystemError(errno, "%s", _("ethtool ioctl error"));

John