|
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.
|
