| Message ID | 20190503170821.260705-2-andre.przywara@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Automatically clean up ghost socket files | expand |
On Fri, May 03, 2019 at 06:08:20PM +0100, Andre Przywara wrote: > When kvmtool (or the host kernel) crashes or gets killed, we cannot > automatically remove the socket file we created for that VM. > A later call of "lkvm list" iterates over all those files and complains > about those "ghost socket files", as there is no one listening on > the other side. Also sometimes the automatic guest name generation > happens to generate the same name again, so an unrelated "lkvm run" > later complains and stops, which is bad for automation. > > As the only code doing a listen() on this socket is kvmtool upon VM > *creation*, such an orphaned socket file will never come back to life, > so we can as well unlink() those sockets in the code. This spares the > user the messages and the burden of doing it herself. > We keep a message in the code to notify the user of this. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > kvm-ipc.c | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/kvm-ipc.c b/kvm-ipc.c > index e07ad105..d9a07595 100644 > --- a/kvm-ipc.c > +++ b/kvm-ipc.c > @@ -101,9 +101,8 @@ int kvm__get_sock_by_instance(const char *name) > > r = connect(s, (struct sockaddr *)&local, len); > if (r < 0 && errno == ECONNREFUSED) { > - /* Tell the user clean ghost socket file */ > - pr_err("\"%s\" could be a ghost socket file, please remove it", > - sock_file); > + /* Clean up the ghost socket file */ > + unlink(local.sun_path); > return r; > } else if (r < 0) { > return r; > @@ -140,6 +139,7 @@ int kvm__enumerate_instances(int (*callback)(const char *name, int fd)) > struct dirent *entry; > int ret = 0; > const char *path; > + int cleaned = 0; > > path = kvm__get_dir(); > > @@ -164,8 +164,11 @@ int kvm__enumerate_instances(int (*callback)(const char *name, int fd)) > > *p = 0; > sock = kvm__get_sock_by_instance(entry->d_name); > - if (sock < 0) > + if (sock < 0) { > + if (errno == ECONNREFUSED) > + cleaned++; This is fragile, because you're relying on errno being preserved from the failing connect() call after kvm__get_sock_by_instance() returns. Yes, that's true today, but it would be easy to change kvm__get_sock_by_instance() in future and miss this detail. Maybe just replace the existing print to that it says about each file being removed? Will
diff --git a/kvm-ipc.c b/kvm-ipc.c index e07ad105..d9a07595 100644 --- a/kvm-ipc.c +++ b/kvm-ipc.c @@ -101,9 +101,8 @@ int kvm__get_sock_by_instance(const char *name) r = connect(s, (struct sockaddr *)&local, len); if (r < 0 && errno == ECONNREFUSED) { - /* Tell the user clean ghost socket file */ - pr_err("\"%s\" could be a ghost socket file, please remove it", - sock_file); + /* Clean up the ghost socket file */ + unlink(local.sun_path); return r; } else if (r < 0) { return r; @@ -140,6 +139,7 @@ int kvm__enumerate_instances(int (*callback)(const char *name, int fd)) struct dirent *entry; int ret = 0; const char *path; + int cleaned = 0; path = kvm__get_dir(); @@ -164,8 +164,11 @@ int kvm__enumerate_instances(int (*callback)(const char *name, int fd)) *p = 0; sock = kvm__get_sock_by_instance(entry->d_name); - if (sock < 0) + if (sock < 0) { + if (errno == ECONNREFUSED) + cleaned++; continue; + } ret = callback(entry->d_name, sock); close(sock); if (ret < 0) @@ -175,6 +178,9 @@ int kvm__enumerate_instances(int (*callback)(const char *name, int fd)) closedir(dir); + if (cleaned) + pr_info("Removed %d ghost socket files", cleaned); + return ret; }
When kvmtool (or the host kernel) crashes or gets killed, we cannot automatically remove the socket file we created for that VM. A later call of "lkvm list" iterates over all those files and complains about those "ghost socket files", as there is no one listening on the other side. Also sometimes the automatic guest name generation happens to generate the same name again, so an unrelated "lkvm run" later complains and stops, which is bad for automation. As the only code doing a listen() on this socket is kvmtool upon VM *creation*, such an orphaned socket file will never come back to life, so we can as well unlink() those sockets in the code. This spares the user the messages and the burden of doing it herself. We keep a message in the code to notify the user of this. Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- kvm-ipc.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)