Revert "utils: Remove the logging of errors from virNetDevSendEthtoolIoctl"

This reverts commit 6f2a0198e9.

This commit removed error reporting from virNetDevSendEthtoolIoctl
pushing responsibility onto the callers. This is wrong, however,
since virNetDevSendEthtoolIoctl calls virNetDevSetupControl
which can still report errors. So as a result virNetDevSendEthtoolIoctl
may or may not report errors depending on which bit of it fails, and as
a result callers now overwrite some errors.

It also introduced a regression causing unprivileged libvirtd to
spew error messages to the console due to inability to query the
NIC features, an error which was previously ignored.

virNetDevSetupControlFull:148 : Cannot open network interface control socket: Operation not permitted
virNetDevFeatureAvailable:3062 : Cannot get device wlp3s0 flags: Operation not permitted
virNetDevSetupControlFull:148 : Cannot open network interface control socket: Operation not permitted
virNetDevFeatureAvailable:3062 : Cannot get device wlp3s0 flags: Operation not permitted
virNetDevSetupControlFull:148 : Cannot open network interface control socket: Operation not permitted
virNetDevFeatureAvailable:3062 : Cannot get device wlp3s0 flags: Operation not permitted
virNetDevSetupControlFull:148 : Cannot open network interface control socket: Operation not permitted
virNetDevFeatureAvailable:3062 : Cannot get device wlp3s0 flags: Operation not permitted

Looking back at the original posting I see no explanation of why
thsi refactoring was needed, so reverting the clearly broken
error reporting logic looks like the best option.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
This commit is contained in:
Daniel P. Berrange 2015-11-03 14:17:42 +00:00 committed by John Ferlan
parent afb792bd38
commit a3f63b853c

View File

@ -3151,15 +3151,39 @@ static int
virNetDevSendEthtoolIoctl(const char *ifname, void *cmd) virNetDevSendEthtoolIoctl(const char *ifname, void *cmd)
{ {
int ret = -1; int ret = -1;
int fd; int sock = -1;
struct ifreq ifr; virIfreq ifr;
if ((fd = virNetDevSetupControl(ifname, &ifr)) < 0) sock = socket(AF_LOCAL, SOCK_DGRAM, 0);
return ret; if (sock < 0) {
virReportSystemError(errno, "%s", _("Cannot open control socket"));
goto cleanup;
}
memset(&ifr, 0, sizeof(ifr));
strcpy(ifr.ifr_name, ifname);
ifr.ifr_data = cmd; ifr.ifr_data = cmd;
ret = ioctl(fd, SIOCETHTOOL, &ifr); ret = ioctl(sock, SIOCETHTOOL, &ifr);
if (ret != 0) {
switch (errno) {
case EPERM:
VIR_DEBUG("ethtool ioctl: permission denied");
break;
case EINVAL:
VIR_DEBUG("ethtool ioctl: invalid request");
break;
case EOPNOTSUPP:
VIR_DEBUG("ethtool ioctl: request not supported");
break;
default:
virReportSystemError(errno, "%s", _("ethtool ioctl error"));
goto cleanup;
}
}
VIR_FORCE_CLOSE(fd); cleanup:
if (sock)
VIR_FORCE_CLOSE(sock);
return ret; return ret;
} }
@ -3176,12 +3200,12 @@ virNetDevSendEthtoolIoctl(const char *ifname, void *cmd)
static int static int
virNetDevFeatureAvailable(const char *ifname, struct ethtool_value *cmd) virNetDevFeatureAvailable(const char *ifname, struct ethtool_value *cmd)
{ {
int ret = -1;
cmd = (void*)cmd; cmd = (void*)cmd;
if (virNetDevSendEthtoolIoctl(ifname, cmd) < 0) { if (!virNetDevSendEthtoolIoctl(ifname, cmd))
virReportSystemError(errno, _("Cannot get device %s flags"), ifname); ret = cmd->data > 0 ? 1 : 0;
return -1; return ret;
}
return cmd->data > 0 ? 1 : 0;
} }
@ -3198,12 +3222,12 @@ virNetDevFeatureAvailable(const char *ifname, struct ethtool_value *cmd)
static int static int
virNetDevGFeatureAvailable(const char *ifname, struct ethtool_gfeatures *cmd) virNetDevGFeatureAvailable(const char *ifname, struct ethtool_gfeatures *cmd)
{ {
int ret = -1;
cmd = (void*)cmd; cmd = (void*)cmd;
if (virNetDevSendEthtoolIoctl(ifname, cmd) < 0) { if (!virNetDevSendEthtoolIoctl(ifname, cmd))
virReportSystemError(errno, _("Cannot get device %s generic features"), ifname); ret = FEATURE_BIT_IS_SET(cmd->features, TX_UDP_TNL, active);
return -1; return ret;
}
return FEATURE_BIT_IS_SET(cmd->features, TX_UDP_TNL, active);
} }
# endif # endif