[libvirt] [PATCH v2] daemon: Unlink unix socket paths on shutdown

[Eric Blake]

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 at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org