On 08/01/2011 09:34 AM, Osier Yang wrote: > This patch introduces a internal RPC API "virNetServerClose", which > is standalone with "virNetServerFree". it closes all the socket fds, > and unlinks the unix socket paths, regardless of whether the socket > is still referenced or not. > > This is to address regression bug: > https://bugzilla.redhat.com/show_bug.cgi?id=725702 > +++ b/src/rpc/virnetserver.c > @@ -794,3 +794,15 @@ void virNetServerFree(virNetServerPtr srv) > virMutexDestroy(&srv->lock); > VIR_FREE(srv); > } > + > +void virNetServerClose(virNetServerPtr srv) > +{ > + int i; > + > + if (!srv) > + return; > + > + for (i = 0; i < srv->nservices; i++) { > + virNetServerServiceClose(srv->services[i]); > + } Don't we need locking at every level of the chain? That is, srv->nservices involves reading srv, so srv must be locked, just as much as: > + > +void virNetServerServiceClose(virNetServerServicePtr svc) > +{ > + int i; > + > + if (!svc) > + return; > + > + for (i = 0; i < svc->nsocks; i++) { > + virNetSocketClose(svc->socks[i]); svc->nsocks, as well as... > +void virNetSocketClose(virNetSocketPtr sock) > +{ > + if (!sock) > + return; > + > + virMutexLock(&sock->lock); sock, but you only locked sock. > + > + VIR_FORCE_CLOSE(sock->fd); > + > +#ifdef HAVE_SYS_UN_H > + /* If a server socket, then unlink UNIX path */ > + if (!sock->client && > + sock->localAddr.data.sa.sa_family == AF_UNIX && > + sock->localAddr.data.un.sun_path[0] != '\0') { > + if (unlink(sock->localAddr.data.un.sun_path) == 0) > + sock->localAddr.data.un.sun_path[0] = '\0'; > + } But I think this change is right. -- Eric Blake eblake@xxxxxxxxxx +1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list