Re: [libvirt] [PATCH] daemon: Unlink unix sockets at daemon shutdown

["Daniel P. Berrange" <berrange redhat com>]

On Wed, Jul 27, 2011 at 05:34:19PM +0800, Osier Yang wrote:
> 于 2011年07月27日 18:07, Osier Yang 写道:
> > ---
> >  daemon/libvirtd.c |    6 ++++++
> >  1 files changed, 6 insertions(+), 0 deletions(-)
> >
> > diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
> > index 9e044e2..61f5a2d 100644
> > --- a/daemon/libvirtd.c
> > +++ b/daemon/libvirtd.c
> > @@ -1572,6 +1572,12 @@ cleanup:
> >      if (pid_file)
> >          unlink (pid_file);
> >  
> > +    if (sock_file && sock_file[0] != '@')
> > +        unlink(sock_file);
> > +
> > +    if (sock_file_ro && sock_file_ro[0] != '@')
> > +        unlink(sock_file_ro);
> > +

NACK to this - cleanup should be done in the RPC classes

> With this patch, the socket files could be removed successfully.
> 
> But it looks to me virNetSocketFree already unlink the socket files,
> it's weired
> the unix socket files still exists after daemon shutdown. So, this patch
> might be
> not quite right.

Yes, that suggests that virNetServerFree or something that
it calls is not cleaning up properly. It could be a missing
free somewhere, or a reference being held open.

It might be desirable to add an explicit virNetServerClose()
API which closes all sockets & unlinks them, separately
from virNetServerFree, so we ensure cleanup even in the event
of a reference count being leaked

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|