All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andre Przywara <andre.przywara@arm.com>
To: Will Deacon <will.deacon@arm.com>
Cc: kvm@vger.kernel.org
Subject: [PATCH kvmtool 2/2] run: Check for ghost socket file upon VM creation
Date: Fri,  3 May 2019 18:08:21 +0100	[thread overview]
Message-ID: <20190503170821.260705-3-andre.przywara@arm.com> (raw)
In-Reply-To: <20190503170821.260705-1-andre.przywara@arm.com>

Kvmtool creates a (debug) UNIX socket file for each VM, using its
(possibly auto-generated) name as the filename. There is a check using
access(), which bails out with an error message if a socket with that
name already exists.

Aside from this check being unnecessary, as the bind() call later would
complain as well, this is also racy. But more annoyingly the bail out is
not needed most of the time: an existing socket inode is most likely just
an orphaned leftover from a previous kvmtool run, which just failed to
remove that file, because of a crash, for instance.

Upon finding such a collision, let's first try to connect to that socket,
to detect if there is still a kvmtool instance listening on the other
end. If that fails, this socket will never come back to life, so we can
safely clean it up and reuse the name for the new guest.
However if the connect() succeeds, there is an actual live kvmtool
instance using this name, so not proceeding is the only option.
This should never happen with the (PID based) automatically generated
names, though.

This avoids an annoying (and not helpful) error message and helps
automated kvmtool runs to proceed in more cases.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 kvm-ipc.c | 30 ++++++++++++++++++++++++++----
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/kvm-ipc.c b/kvm-ipc.c
index d9a07595..06909171 100644
--- a/kvm-ipc.c
+++ b/kvm-ipc.c
@@ -43,10 +43,6 @@ static int kvm__create_socket(struct kvm *kvm)
 
 	snprintf(full_name, sizeof(full_name), "%s/%s%s",
 		 kvm__get_dir(), kvm->cfg.guest_name, KVM_SOCK_SUFFIX);
-	if (access(full_name, F_OK) == 0) {
-		pr_err("Socket file %s already exist", full_name);
-		return -EEXIST;
-	}
 
 	s = socket(AF_UNIX, SOCK_STREAM, 0);
 	if (s < 0) {
@@ -58,6 +54,32 @@ static int kvm__create_socket(struct kvm *kvm)
 	strlcpy(local.sun_path, full_name, sizeof(local.sun_path));
 	len = strlen(local.sun_path) + sizeof(local.sun_family);
 	r = bind(s, (struct sockaddr *)&local, len);
+	/* Check for an existing socket file */
+	if (r < 0 && errno == EADDRINUSE) {
+		r = connect(s, (struct sockaddr *)&local, len);
+		if (r == 0) {
+			/*
+			 * If we could connect, there is already a guest
+			 * using this same name. This should not happen
+			 * for PID derived names, but could happen for user
+			 * provided guest names.
+			 */
+			pr_err("Guest socket file %s already exists.",
+			       full_name);
+			r = -EEXIST;
+			goto fail;
+		}
+		if (errno == ECONNREFUSED) {
+			/*
+			 * This is a ghost socket file, with no-one listening
+			 * on the other end. Since kvmtool will only bind
+			 * above when creating a new guest, there is no
+			 * danger in just removing the file and re-trying.
+			 */
+			unlink(full_name);
+			r = bind(s, (struct sockaddr *)&local, len);
+		}
+	}
 	if (r < 0) {
 		perror("bind");
 		goto fail;
-- 
2.17.1


  parent reply	other threads:[~2019-05-03 17:08 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-03 17:08 [PATCH kvmtool 0/2] Automatically clean up ghost socket files Andre Przywara
2019-05-03 17:08 ` [PATCH kvmtool 1/2] list: Clean " Andre Przywara
2019-05-24 16:27   ` Will Deacon
2019-05-03 17:08 ` Andre Przywara [this message]
2019-05-24 16:27   ` [PATCH kvmtool 2/2] run: Check for ghost socket file upon VM creation Will Deacon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190503170821.260705-3-andre.przywara@arm.com \
    --to=andre.przywara@arm.com \
    --cc=kvm@vger.kernel.org \
    --cc=will.deacon@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.