Plz ignore that. Posted wrong patch.
>>> "Chun Yan Liu" <cyliu@xxxxxxxxxx> 2011/1/25 10:49 >>>
Ian, about the include header file problem, I did find it some day when I tried to make stubdom. I thought I only checked make tools before. Sorry for that. Actually, stubdom libxc are soft links to tools/libxc/, but it uses newlibc contained in itself, not a standard gcc as in 'make tools', that newlibc does not export a flock interface. So, I think we should not use flock, use fcntl instead. Following is a patch using fcntl, I've tested it. Please have a check. Thanks.
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 Mon Jan 24 23:15:57 2011 +0800 @@ -16,19 +16,21 @@ #include "xc_private.h" #include "xenguest.h" +#include <unistd.h> +#include <fcntl.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]; + struct flock fl; 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 +38,11 @@ return -EINVAL; } umask(mask); - snprintf(buf, sizeof(buf), "%10ld", (long)getpid()); + memset(&fl,0,sizeof(fl)); + fl.l_type = F_WRLCK; + fl.l_whence = SEEK_SET; + rc = fcntl(fd, F_SETLK, &fl); - rc = write_exact(fd, buf, strlen(buf)); close(fd); return rc; @@ -46,9 +50,9 @@ static int unlock_suspend_event(xc_interface *xch, int domid) { - int fd, pid, n; - char buf[128]; + int fd, rc; char suspend_file[256]; + struct flock fl; snprintf(suspend_file, sizeof(suspend_file), "%s_%d_lock.d", SUSPEND_LOCK_FILE, domid); @@ -57,22 +61,17 @@ if (fd < 0) return -EINVAL; - n = read(fd, buf, 127); + memset(&fl,0,sizeof(fl)); + fl.l_type = F_UNLCK; + fl.l_whence = SEEK_SET; + rc = fcntl(fd, F_SETLK, &fl); 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 +126,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; }
>>> Chun Yan Liu <cyliu@xxxxxxxxxx> 12/22/10 2:32 PM >>> Sorry for delay, following is an explanation:
>> --- 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
>That looks like a mistake.
I wonder if you think the header file has something wrong?
flock is defined in <sys/file.h>. To use it, need to include that header file.
>> @@ -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; >> }
>What is this change for ?
if (suspend_evtchn != -1) part is unnecessary and might leave an orphan lock file.
Only suspend_evtchn > 0 means we got the lock successfully, all other values means we failed to bind suspend event channel, so we should do cleanup work, including: the lock file we created before should be removed. Otherwise, in xc_save, if suspend_evtchn = -1, there will be no chance to remove the lock file.
I noticed this part was added by Jiang, Yunhong, I've talked with him before, he also thought that the if (suspend_evtchn != -1) part should be removed altogether.
|