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. -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list