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

[Martin Kletzander]

On Sat, Aug 15, 2015 at 12:04:04PM +0300, 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 |   44 +++++++++++++-------------------------------
> 1 files changed, 13 insertions(+), 31 deletions(-)
>
>diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
>index 2f3690e..cf79e8d 100644
>--- a/src/util/virnetdev.c
>+++ b/src/util/virnetdev.c
>@@ -3032,11 +3032,10 @@ static int
> virNetDevSendEthtoolIoctl(const char *ifname, void *cmd)
> {
>     int ret = -1;
>-    int sock = -1;
>+    int sock;
>     virIfreq ifr;
>
>-    sock = socket(AF_LOCAL, SOCK_DGRAM, 0);
>-    if (sock < 0) {
>+    if ((sock = socket(AF_LOCAL, SOCK_DGRAM, 0)) < 0) {
>         virReportSystemError(errno, "%s", _("Cannot open control socket"));
>         goto cleanup;

Here you still report error from this function.

>     }
>@@ -3045,26 +3044,9 @@ virNetDevSendEthtoolIoctl(const char *ifname, void *cmd)
>     strcpy(ifr.ifr_name, ifname);
>     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;
>-        }
>-    }
>
>  cleanup:
>-    if (sock)
>-        VIR_FORCE_CLOSE(sock);
>+    VIR_FORCE_CLOSE(sock);
>     return ret;
> }
>
>@@ -3081,12 +3063,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 +3085,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;

And here and above you rewrite it.  I suggest just using
virReportError in the virNetDevSendEthtoolIoctl() function based on
what happened and don't rewrite it in callers.

>+    }
>+    return FEATURE_BIT_IS_SET(cmd->features, TX_UDP_TNL, active);
> }
> # endif
>
>--
>1.7.1
>
>--
>libvir-list mailing list
>libvir-list at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150817/5fb226bb/attachment-0001.sig>