|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-changelog] [xen master] libxc: suspend: Fix suspend event channel locking
commit 7b0360b76116d9b104798f1f533548d436ca50e6
Author: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
AuthorDate: Wed Dec 11 16:29:38 2013 +0000
Commit: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
CommitDate: Mon Mar 17 15:53:59 2014 +0000
libxc: suspend: Fix suspend event channel locking
Use fcntl F_SETLK, rather than writing our pid into a "lock" file.
That way if we crash we don't leave the lockfile lying about. Callers
now need to keep the fd for our lockfile. (We don't use flock because
we don't want anyone who inherits this fd across fork to end up with a
handle onto the lock.)
While we are here:
* Move the lockfile to /var/run/xen
* De-duplicate the calculation of the pathname
* Compute the buffer size for the pathname so that it will definitely
not overrun (and use the computed value everywhere)
* Fix various error handling bugs
Signed-off-by: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
CC: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
Acked-by: Shriram Rajagopalan <rshriram@xxxxxxxxx>
---
tools/libxc/xc_suspend.c | 160 +++++++++++++-------
tools/libxc/xenguest.h | 16 ++-
tools/libxl/libxl_dom.c | 6 +-
tools/libxl/libxl_internal.h | 1 +
tools/misc/xen-hptool.c | 19 ++-
tools/python/xen/lowlevel/checkpoint/checkpoint.h | 2 +-
.../python/xen/lowlevel/checkpoint/libcheckpoint.c | 7 +-
tools/xcutils/xc_save.c | 8 +-
8 files changed, 150 insertions(+), 69 deletions(-)
diff --git a/tools/libxc/xc_suspend.c b/tools/libxc/xc_suspend.c
index eed3be2..84ee139 100644
--- a/tools/libxc/xc_suspend.c
+++ b/tools/libxc/xc_suspend.c
@@ -14,65 +14,115 @@
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
USA
*/
+#include <unistd.h>
+#include <fcntl.h>
+
#include "xc_private.h"
#include "xenguest.h"
-#define SUSPEND_LOCK_FILE "/var/lib/xen/suspend_evtchn"
-static int lock_suspend_event(xc_interface *xch, int domid)
+#define SUSPEND_LOCK_FILE "/var/run/xen/suspend-evtchn-%d.lock"
+
+/*
+ * locking
+ */
+
+#define ERR(x) do{ \
+ ERROR("Can't " #x " lock file for suspend event channel %s: %s\n", \
+ suspend_file, strerror(errno)); \
+ goto err; \
+}while(0)
+
+#define SUSPEND_FILE_BUFLEN (sizeof(SUSPEND_LOCK_FILE) + 10)
+
+static void get_suspend_file(char buf[SUSPEND_FILE_BUFLEN], int domid)
{
- int fd, rc;
- mode_t mask;
- char buf[128];
- char suspend_file[256];
-
- snprintf(suspend_file, sizeof(suspend_file), "%s_%d_lock.d",
- SUSPEND_LOCK_FILE, domid);
- mask = umask(022);
- fd = open(suspend_file, O_CREAT | O_EXCL | O_RDWR, 0666);
- if (fd < 0)
- {
- ERROR("Can't create lock file for suspend event channel %s\n",
- suspend_file);
- return -EINVAL;
+ snprintf(buf, sizeof(buf), SUSPEND_LOCK_FILE, domid);
+}
+
+static int lock_suspend_event(xc_interface *xch, int domid, int *lockfd)
+{
+ int fd = -1, r;
+ char suspend_file[SUSPEND_FILE_BUFLEN];
+ struct stat ours, theirs;
+ struct flock fl;
+
+ get_suspend_file(suspend_file, domid);
+
+ *lockfd = -1;
+
+ for (;;) {
+ if (fd >= 0)
+ close (fd);
+
+ fd = open(suspend_file, O_CREAT | O_RDWR, 0600);
+ if (fd < 0)
+ ERR("create");
+
+ r = fcntl(fd, F_SETFD, FD_CLOEXEC);
+ if (r)
+ ERR("fcntl F_SETFD FD_CLOEXEC");
+
+ memset(&fl, 0, sizeof(fl));
+ fl.l_type = F_WRLCK;
+ fl.l_whence = SEEK_SET;
+ fl.l_len = 1;
+ r = fcntl(fd, F_SETLK, &fl);
+ if (r)
+ ERR("fcntl F_SETLK");
+
+ r = fstat(fd, &ours);
+ if (r)
+ ERR("fstat");
+
+ r = stat(suspend_file, &theirs);
+ if (r) {
+ if (errno == ENOENT)
+ /* try again */
+ continue;
+ ERR("stat");
+ }
+
+ if (ours.st_ino != theirs.st_ino)
+ /* someone else must have removed it while we were locking it */
+ continue;
+
+ break;
}
- umask(mask);
- snprintf(buf, sizeof(buf), "%10ld", (long)getpid());
- rc = write_exact(fd, buf, strlen(buf));
- close(fd);
+ *lockfd = fd;
+ return 0;
- return rc;
+ err:
+ if (fd >= 0)
+ close(fd);
+
+ return -1;
}
-static int unlock_suspend_event(xc_interface *xch, int domid)
+static int unlock_suspend_event(xc_interface *xch, int domid, int *lockfd)
{
- int fd, pid, n;
- char buf[128];
- char suspend_file[256];
+ int r;
+ char suspend_file[SUSPEND_FILE_BUFLEN];
- snprintf(suspend_file, sizeof(suspend_file), "%s_%d_lock.d",
- SUSPEND_LOCK_FILE, domid);
- fd = open(suspend_file, O_RDWR);
+ if (*lockfd < 0)
+ return 0;
- if (fd < 0)
- return -EINVAL;
+ get_suspend_file(suspend_file, domid);
- n = read(fd, buf, 127);
+ r = unlink(suspend_file);
+ if (r)
+ ERR("unlink");
- close(fd);
+ r = close(*lockfd);
+ *lockfd = -1;
+ if (r)
+ ERR("close");
- if (n > 0)
- {
- sscanf(buf, "%d", &pid);
- /* We are the owner, so we can simply delete the file */
- if (pid == getpid())
- {
- unlink(suspend_file);
- return 0;
- }
- }
+ err:
+ if (*lockfd >= 0)
+ close(*lockfd);
- return -EPERM;
+ return -1;
}
int xc_await_suspend(xc_interface *xch, xc_evtchn *xce, int suspend_evtchn)
@@ -94,20 +144,26 @@ int xc_await_suspend(xc_interface *xch, xc_evtchn *xce,
int suspend_evtchn)
return 0;
}
-int xc_suspend_evtchn_release(xc_interface *xch, xc_evtchn *xce, int domid,
int suspend_evtchn)
+/* Internal callers are allowed to call this with suspend_evtchn<0
+ * but *lockfd>0. */
+int xc_suspend_evtchn_release(xc_interface *xch, xc_evtchn *xce,
+ int domid, int suspend_evtchn, int *lockfd)
{
if (suspend_evtchn >= 0)
xc_evtchn_unbind(xce, suspend_evtchn);
- return unlock_suspend_event(xch, domid);
+ return unlock_suspend_event(xch, domid, lockfd);
}
-int xc_suspend_evtchn_init_sane(xc_interface *xch, xc_evtchn *xce, int domid,
int port)
+int xc_suspend_evtchn_init_sane(xc_interface *xch, xc_evtchn *xce,
+ int domid, int port, int *lockfd)
{
int rc, suspend_evtchn = -1;
- if (lock_suspend_event(xch, domid))
- return -EINVAL;
+ if (lock_suspend_event(xch, domid, lockfd)) {
+ errno = EINVAL;
+ goto cleanup;
+ }
suspend_evtchn = xc_evtchn_bind_interdomain(xce, domid, port);
if (suspend_evtchn < 0) {
@@ -124,17 +180,17 @@ int xc_suspend_evtchn_init_sane(xc_interface *xch,
xc_evtchn *xce, int domid, in
return suspend_evtchn;
cleanup:
- if (suspend_evtchn != -1)
- xc_suspend_evtchn_release(xch, xce, domid, suspend_evtchn);
+ xc_suspend_evtchn_release(xch, xce, domid, suspend_evtchn, lockfd);
return -1;
}
-int xc_suspend_evtchn_init_exclusive(xc_interface *xch, xc_evtchn *xce, int
domid, int port)
+int xc_suspend_evtchn_init_exclusive(xc_interface *xch, xc_evtchn *xce,
+ int domid, int port, int *lockfd)
{
int suspend_evtchn;
- suspend_evtchn = xc_suspend_evtchn_init_sane(xch, xce, domid, port);
+ suspend_evtchn = xc_suspend_evtchn_init_sane(xch, xce, domid, port,
lockfd);
if (suspend_evtchn < 0)
return suspend_evtchn;
diff --git a/tools/libxc/xenguest.h b/tools/libxc/xenguest.h
index ce5456c..1f216cd 100644
--- a/tools/libxc/xenguest.h
+++ b/tools/libxc/xenguest.h
@@ -254,13 +254,19 @@ int xc_hvm_build_target_mem(xc_interface *xch,
int target,
const char *image_name);
-int xc_suspend_evtchn_release(xc_interface *xch, xc_evtchn *xce, int domid,
int suspend_evtchn);
+/*
+ * Sets *lockfd to -1.
+ * Has deallocated everything even on error.
+ */
+int xc_suspend_evtchn_release(xc_interface *xch, xc_evtchn *xce, int domid,
int suspend_evtchn, int *lockfd);
/**
* This function eats the initial notification.
* xce must not be used for anything else
+ * See xc_suspend_evtchn_init_sane re lockfd.
*/
-int xc_suspend_evtchn_init_exclusive(xc_interface *xch, xc_evtchn *xce, int
domid, int port);
+int xc_suspend_evtchn_init_exclusive(xc_interface *xch, xc_evtchn *xce,
+ int domid, int port, int *lockfd);
/* xce must not be used for anything else */
int xc_await_suspend(xc_interface *xch, xc_evtchn *xce, int suspend_evtchn);
@@ -268,8 +274,12 @@ int xc_await_suspend(xc_interface *xch, xc_evtchn *xce,
int suspend_evtchn);
/**
* The port will be signaled immediately after this call
* The caller should check the domain status and look for the next event
+ * On success, *lockfd will be set to >=0 and *lockfd must be preserved
+ * and fed to xc_suspend_evtchn_release. (On error *lockfd is
+ * undefined and xc_suspend_evtchn_release is not allowed.)
*/
-int xc_suspend_evtchn_init_sane(xc_interface *xch, xc_evtchn *xce, int domid,
int port);
+int xc_suspend_evtchn_init_sane(xc_interface *xch, xc_evtchn *xce,
+ int domid, int port, int *lockfd);
int xc_get_bit_size(xc_interface *xch,
const char *image_name, const char *cmdline,
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 4b42856..48a4b8e 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1340,6 +1340,7 @@ void libxl__domain_suspend(libxl__egc *egc,
libxl__domain_suspend_state *dss)
| (dss->hvm ? XCFLAGS_HVM : 0);
dss->suspend_eventchn = -1;
+ dss->guest_evtchn_lockfd = -1;
dss->guest_responded = 0;
dss->dm_savefile = libxl__device_model_savefile(gc, domid);
@@ -1358,7 +1359,8 @@ void libxl__domain_suspend(libxl__egc *egc,
libxl__domain_suspend_state *dss)
if (port >= 0) {
dss->suspend_eventchn =
- xc_suspend_evtchn_init_exclusive(CTX->xch, dss->xce,
dss->domid, port);
+ xc_suspend_evtchn_init_exclusive(CTX->xch, dss->xce,
+ dss->domid, port, &dss->guest_evtchn_lockfd);
if (dss->suspend_eventchn < 0)
LOG(WARN, "Suspend event channel initialization failed");
@@ -1522,7 +1524,7 @@ static void domain_suspend_done(libxl__egc *egc,
if (dss->suspend_eventchn > 0)
xc_suspend_evtchn_release(CTX->xch, dss->xce, domid,
- dss->suspend_eventchn);
+ dss->suspend_eventchn, &dss->guest_evtchn_lockfd);
if (dss->xce != NULL)
xc_evtchn_close(dss->xce);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 1914d1b..d15a4b2 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2439,6 +2439,7 @@ struct libxl__domain_suspend_state {
/* private */
xc_evtchn *xce; /* event channel handle */
int suspend_eventchn;
+ int guest_evtchn_lockfd;
int hvm;
int xcflags;
int guest_responded;
diff --git a/tools/misc/xen-hptool.c b/tools/misc/xen-hptool.c
index 1923be9..8aac51c 100644
--- a/tools/misc/xen-hptool.c
+++ b/tools/misc/xen-hptool.c
@@ -98,10 +98,13 @@ static int hp_mem_query_func(int argc, char *argv[])
extern int xs_suspend_evtchn_port(int domid);
-static int suspend_guest(xc_interface *xch, xc_evtchn *xce, int domid, int
*evtchn)
+static int suspend_guest(xc_interface *xch, xc_evtchn *xce, int domid,
+ int *evtchn, int *lockfd)
{
int port, rc, suspend_evtchn = -1;
+ *lockfd = -1;
+
if (!evtchn)
return -1;
@@ -111,7 +114,8 @@ static int suspend_guest(xc_interface *xch, xc_evtchn *xce,
int domid, int *evtc
fprintf(stderr, "DOM%d: No suspend port, try live migration\n", domid);
goto failed;
}
- suspend_evtchn = xc_suspend_evtchn_init_exclusive(xch, xce, domid, port);
+ suspend_evtchn = xc_suspend_evtchn_init_exclusive(xch, xce, domid,
+ port, lockfd);
if (suspend_evtchn < 0)
{
fprintf(stderr, "Suspend evtchn initialization failed\n");
@@ -134,7 +138,8 @@ static int suspend_guest(xc_interface *xch, xc_evtchn *xce,
int domid, int *evtc
failed:
if (suspend_evtchn != -1)
- xc_suspend_evtchn_release(xch, xce, domid, suspend_evtchn);
+ xc_suspend_evtchn_release(xch, xce, domid,
+ suspend_evtchn, lockfd);
return -1;
}
@@ -192,7 +197,7 @@ static int hp_mem_offline_func(int argc, char *argv[])
}
else if (status & PG_OFFLINE_OWNED)
{
- int result, suspend_evtchn = -1;
+ int result, suspend_evtchn = -1, suspend_lockfd = -1;
xc_evtchn *xce;
xce = xc_evtchn_open(NULL, 0);
@@ -204,7 +209,8 @@ static int hp_mem_offline_func(int argc, char *argv[])
}
domid = status >> PG_OFFLINE_OWNER_SHIFT;
- if (suspend_guest(xch, xce, domid, &suspend_evtchn))
+ if (suspend_guest(xch, xce, domid,
+ &suspend_evtchn, &suspend_lockfd))
{
fprintf(stderr, "Failed to suspend guest %d for"
" mfn %lx\n", domid, mfn);
@@ -230,7 +236,8 @@ static int hp_mem_offline_func(int argc, char *argv[])
mfn, domid);
}
xc_domain_resume(xch, domid, 1);
- xc_suspend_evtchn_release(xch, xce, domid, suspend_evtchn);
+ xc_suspend_evtchn_release(xch, xce, domid,
+ suspend_evtchn, &suspend_lockfd);
xc_evtchn_close(xce);
}
break;
diff --git a/tools/python/xen/lowlevel/checkpoint/checkpoint.h
b/tools/python/xen/lowlevel/checkpoint/checkpoint.h
index 187d9d7..2414956 100644
--- a/tools/python/xen/lowlevel/checkpoint/checkpoint.h
+++ b/tools/python/xen/lowlevel/checkpoint/checkpoint.h
@@ -27,7 +27,7 @@ typedef struct {
checkpoint_domtype domtype;
int fd;
- int suspend_evtchn;
+ int suspend_evtchn, suspend_lockfd;
char* errstr;
diff --git a/tools/python/xen/lowlevel/checkpoint/libcheckpoint.c
b/tools/python/xen/lowlevel/checkpoint/libcheckpoint.c
index 817d272..74ca062 100644
--- a/tools/python/xen/lowlevel/checkpoint/libcheckpoint.c
+++ b/tools/python/xen/lowlevel/checkpoint/libcheckpoint.c
@@ -57,6 +57,7 @@ void checkpoint_init(checkpoint_state* s)
s->fd = -1;
s->suspend_evtchn = -1;
+ s->suspend_lockfd = -1;
s->errstr = NULL;
@@ -360,7 +361,8 @@ static int setup_suspend_evtchn(checkpoint_state* s)
return -1;
}
- s->suspend_evtchn = xc_suspend_evtchn_init_exclusive(s->xch, s->xce,
s->domid, port);
+ s->suspend_evtchn = xc_suspend_evtchn_init_exclusive(s->xch, s->xce,
+ s->domid, port, &s->suspend_lockfd);
if (s->suspend_evtchn < 0) {
s->errstr = "failed to bind suspend event channel";
return -1;
@@ -377,7 +379,8 @@ static void release_suspend_evtchn(checkpoint_state *s)
{
/* TODO: teach xen to clean up if port is unbound */
if (s->xce != NULL && s->suspend_evtchn >= 0) {
- xc_suspend_evtchn_release(s->xch, s->xce, s->domid, s->suspend_evtchn);
+ xc_suspend_evtchn_release(s->xch, s->xce, s->domid,
+ s->suspend_evtchn, &s->suspend_lockfd);
s->suspend_evtchn = -1;
}
}
diff --git a/tools/xcutils/xc_save.c b/tools/xcutils/xc_save.c
index aaa09b0..01adc56 100644
--- a/tools/xcutils/xc_save.c
+++ b/tools/xcutils/xc_save.c
@@ -164,7 +164,7 @@ int
main(int argc, char **argv)
{
unsigned int maxit, max_f, lflags;
- int io_fd, ret, port;
+ int io_fd, ret, port, suspend_lockfd = -1;
struct save_callbacks callbacks;
xentoollog_level lvl;
xentoollog_logger *l;
@@ -199,7 +199,8 @@ main(int argc, char **argv)
else
{
si.suspend_evtchn =
- xc_suspend_evtchn_init_exclusive(si.xch, si.xce, si.domid, port);
+ xc_suspend_evtchn_init_exclusive(si.xch, si.xce, si.domid,
+ port, &suspend_lockfd);
if (si.suspend_evtchn < 0)
warnx("suspend event channel initialization failed, "
@@ -213,7 +214,8 @@ main(int argc, char **argv)
&callbacks, !!(si.flags & XCFLAGS_HVM), 0);
if (si.suspend_evtchn > 0)
- xc_suspend_evtchn_release(si.xch, si.xce, si.domid, si.suspend_evtchn);
+ xc_suspend_evtchn_release(si.xch, si.xce, si.domid,
+ si.suspend_evtchn, &suspend_lockfd);
if (si.xce > 0)
xc_evtchn_close(si.xce);
--
generated by git-patchbot for /home/xen/git/xen.git#master
_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |