On Mon, Jun 06, 2016 at 09:39:28 +0200, Ján Tomko wrote:
This speeds up node_device_udev driver startup 11x. --- src/util/virnetdev.c | 69 +++++++++++++++++++++++++++++----------------------- 1 file changed, 38 insertions(+), 31 deletions(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 52f02d3..7863e8a 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -3206,20 +3206,11 @@ virNetDevRDMAFeature(const char *ifname, * Returns 0 on success, -1 on failure. */ static int -virNetDevSendEthtoolIoctl(const char *ifname, void *cmd) +virNetDevSendEthtoolIoctl(int fd, struct ifreq *ifr)
You changed arguments but didn't fix the comment that documents them.
{ int ret = -1; - int fd = -1; - struct ifreq ifr; - - /* Ultimately uses AF_PACKET for socket which requires privileged - * daemon support. - */ - if ((fd = virNetDevSetupControl(ifname, &ifr)) < 0) - return ret;
- ifr.ifr_data = cmd; - ret = ioctl(fd, SIOCETHTOOL, &ifr); + ret = ioctl(fd, SIOCETHTOOL, ifr); if (ret != 0) { switch (errno) { case EINVAL: /* kernel doesn't support SIOCETHTOOL */ @@ -3230,12 +3221,10 @@ virNetDevSendEthtoolIoctl(const char *ifname, void *cmd) break; default: virReportSystemError(errno, "%s", _("ethtool ioctl error")); - goto cleanup; + break;
These error reports don't make much sense now, but I guess we can keep them for debugging purposes.
} }
- cleanup: - VIR_FORCE_CLOSE(fd); return ret; }
@@ -3255,10 +3244,10 @@ struct ethtool_to_virnetdev_feature { * Returns 0 if not found, 1 on success, and -1 on failure.
Again. Argument change is not documented.
*/ static bool -virNetDevFeatureAvailable(const char *ifname, struct ethtool_value *cmd) +virNetDevFeatureAvailable(int fd, struct ifreq *ifr, struct ethtool_value *cmd) { - cmd = (void*)cmd; - if (virNetDevSendEthtoolIoctl(ifname, cmd) == 0 && + ifr->ifr_data = (void*)cmd; + if (virNetDevSendEthtoolIoctl(fd, ifr) == 0 && cmd->data > 0) return true; return false;
@@ -3332,10 +3322,12 @@ virNetDevGetEthtoolFeatures(virBitmapPtr bitmap, * Returns 0 if not found, 1 on success, and -1 on failure.
And here too ...
*/ static bool -virNetDevGFeatureAvailable(const char *ifname, struct ethtool_gfeatures *cmd) +virNetDevGFeatureAvailable(int fd, + struct ifreq *ifr, + struct ethtool_gfeatures *cmd) { - cmd = (void*)cmd; - if (virNetDevSendEthtoolIoctl(ifname, cmd) == 0) + ifr->ifr_data = (void*)cmd; + if (virNetDevSendEthtoolIoctl(fd, ifr) == 0) return !!FEATURE_BIT_IS_SET(cmd->features, TX_UDP_TNL, active); return false; } @@ -3343,7 +3335,8 @@ virNetDevGFeatureAvailable(const char *ifname, struct ethtool_gfeatures *cmd)
static int virNetDevGetEthtoolGFeatures(virBitmapPtr bitmap, - const char *ifname) + int fd, + struct ifreq *ifr)
Broken formatting.
{ struct ethtool_gfeatures *g_cmd;
[...]
@@ -3391,14 +3389,23 @@ virNetDevGetFeatures(const char *ifname, return 0; }
- virNetDevGetEthtoolFeatures(*out, ifname); + /* Ultimately uses AF_PACKET for socket which requires privileged + * daemon support. + */ + if ((fd = virNetDevSetupControl(ifname, &ifr)) < 0) + goto cleanup;
- if (virNetDevGetEthtoolGFeatures(*out, ifname) < 0) - return -1; + virNetDevGetEthtoolFeatures(*out, fd, &ifr); + + if (virNetDevGetEthtoolGFeatures(*out, fd, &ifr) < 0) + goto cleanup;
if (virNetDevRDMAFeature(ifname, out) < 0) - return -1; - return 0; + goto cleanup; + + ret = 0; + cleanup:
You forgot to close the socket.
+ return ret; } #else int
ACK with the issues fixed.