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.
|
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|