Re: [libvirt] [PATCH] Revert "utils: Remove the logging of errors from virNetDevSendEthtoolIoctl"
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