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@xxxxxxx>
> ---
> 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
