[libvirt] [PATCH] daemon: Unlink unix sockets at daemon shutdown
--- 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); + VIR_FREE(sock_file); VIR_FREE(sock_file_ro); VIR_FREE(pid_file); -- 1.7.6
于 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); +
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.
VIR_FREE(sock_file); VIR_FREE(sock_file_ro); VIR_FREE(pid_file);
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 :|
于 2011年07月27日 17:51, Daniel P. Berrange 写道:
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 looks to me the "ref" is right from the debug log, (not missed somewehre).
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
Yes, agree.
Daniel
participants (2)
-
Daniel P. Berrange -
Osier Yang