[libvirt] [PATCHv2] utils: Remove the logging of errors from virNetDevSendEthtoolIoctl
This patch remove the logging of errors of ioctl api and instead let the caller to choose what errors to log --- src/util/virnetdev.c | 56 ++++++++++++++----------------------------------- 1 files changed, 16 insertions(+), 40 deletions(-) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 2f3690e..5fcf805 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -3032,39 +3032,15 @@ static int virNetDevSendEthtoolIoctl(const char *ifname, void *cmd) { int ret = -1; - int sock = -1; - virIfreq ifr; - - sock = socket(AF_LOCAL, SOCK_DGRAM, 0); - if (sock < 0) { - virReportSystemError(errno, "%s", _("Cannot open control socket")); - goto cleanup; - } + int fd; + struct ifreq ifr; - memset(&ifr, 0, sizeof(ifr)); - strcpy(ifr.ifr_name, ifname); + if ((fd = virNetDevSetupControl(ifname, &ifr)) < 0) + return ret; ifr.ifr_data = cmd; - 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; - } - } + ret = ioctl(fd, SIOCETHTOOL, &ifr); - cleanup: - if (sock) - VIR_FORCE_CLOSE(sock); + VIR_FORCE_CLOSE(fd); return ret; } @@ -3081,12 +3057,12 @@ virNetDevSendEthtoolIoctl(const char *ifname, void *cmd) static int virNetDevFeatureAvailable(const char *ifname, struct ethtool_value *cmd) { - int ret = -1; - cmd = (void*)cmd; - if (!virNetDevSendEthtoolIoctl(ifname, cmd)) - ret = cmd->data > 0 ? 1 : 0; - return ret; + if (virNetDevSendEthtoolIoctl(ifname, cmd) < 0) { + virReportSystemError(errno, _("Cannot get device %s flags"), ifname); + return -1; + } + return cmd->data > 0 ? 1 : 0; } @@ -3103,12 +3079,12 @@ virNetDevFeatureAvailable(const char *ifname, struct ethtool_value *cmd) static int virNetDevGFeatureAvailable(const char *ifname, struct ethtool_gfeatures *cmd) { - int ret = -1; - cmd = (void*)cmd; - if (!virNetDevSendEthtoolIoctl(ifname, cmd)) - ret = FEATURE_BIT_IS_SET(cmd->features, TX_UDP_TNL, active); - return ret; + if (virNetDevSendEthtoolIoctl(ifname, cmd) < 0) { + virReportSystemError(errno, _("Cannot get device %s generic features"), ifname); + return -1; + } + return FEATURE_BIT_IS_SET(cmd->features, TX_UDP_TNL, active); } # endif -- 1.7.1
On 08/18/2015 04:58 PM, Moshe Levi wrote:
This patch remove the logging of errors of ioctl api and instead let the caller to choose what errors to log --- src/util/virnetdev.c | 56 ++++++++++++++----------------------------------- 1 files changed, 16 insertions(+), 40 deletions(-)
Sorry for the delay - last week was KVM Forum and I'm finally mostly caught up with my emails.. Anyway, this looks a whole lot better than before and "no different" than before where if virNetDevSendEthtoolIoctl returned failure the callers to virNetDevFeatureAvailable and virNetDevGFeatureAvailable still make their virBitmapSetBit calls regardless of whether the call returns 1 or -1. That's (more or less) what Jan and Laine discussed in their review of Laine's similar changes. Anyway, I asked Laine to take a peek before I pushed... John I suppose my one hangup is the EOPNOTSUPP which would seemingly need to be "handled" at some point in the future "if" there is filtering and only because I'm not completely convinced whether someone could be running on an older kernel where perhaps one of the cmd bits isn't available (yet). I guess that will be revealed in followup patches!
participants (2)
-
John Ferlan -
Moshe Levi