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

Osier Yang jyang at redhat.com
Mon Aug 1 16:13:32 UTC 2011


于 2011年08月01日 23:39, 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:

Yes, srv needs to be locked, but svc doesn't have lock.  I'd like push with
locking srv if you are fine with it.

>> +
>> +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.
>




More information about the libvir-list mailing list