Yes. Using fctrl or flock instead can avoid the problem, better than an O_EXCL lock file.
I made a patch to use flock instead, please have a review:
diff -r 3c4c3d48a835 tools/libxc/xc_suspend.c
--- a/tools/libxc/xc_suspend.c Thu Aug 26 11:16:56 2010 +0100
+++ b/tools/libxc/xc_suspend.c Fri Nov 26 19:41:23 2010 +0800
@@ -16,19 +16,19 @@
#include "xc_private.h"
#include "xenguest.h"
+#include <sys/file.h>
#define SUSPEND_LOCK_FILE "/var/lib/xen/suspend_evtchn"
static int lock_suspend_event(xc_interface *xch, 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);
+ fd = open(suspend_file, O_CREAT | O_RDWR, 0666);
if (fd < 0)
{
ERROR("Can't create lock file for suspend event channel %s\n",
@@ -36,9 +36,7 @@
return -EINVAL;
}
umask(mask);
- snprintf(buf, sizeof(buf), "%10ld", (long)getpid());
-
- rc = write_exact(fd, buf, strlen(buf));
+ rc = flock(fd, LOCK_EX | LOCK_NB);
close(fd);
return rc;
@@ -46,8 +44,7 @@
static int unlock_suspend_event(xc_interface *xch, int domid)
{
- int fd, pid, n;
- char buf[128];
+ int fd, rc;
char suspend_file[256];
snprintf(suspend_file, sizeof(suspend_file), "%s_%d_lock.d",
@@ -57,22 +54,13 @@
if (fd < 0)
return -EINVAL;
- n = read(fd, buf, 127);
-
+ rc = flock(fd, LOCK_UN | LOCK_NB);
close(fd);
- 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;
- }
- }
+ if(!rc)
+ unlink(suspend_file);
- return -EPERM;
+ return rc;
}
int xc_await_suspend(xc_interface *xch, int xce, int suspend_evtchn)
@@ -127,8 +115,7 @@
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);
return -1;
}
>>> Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx> 11/25/10 3:03 PM >>>
Chun Yan Liu writes ("[Xen-devel][PATCH]improve suspend_evtchn lock processing"):
> While doing migration, sometimes found suspend lock file was not
> unlinked in previous operations, then there is an obsolete lock file
> in place, which causes the current and later migration cannot get
> lock. That happens seldomly but do happen.
I think we should be using fcntl or flock, not an O_EXCL lockfile.
This kind of messing about with pids reduces the proportion of
situations where things go wrong, but it cannot be made race-free.
Ian.
|
