* [PATCH] xenconsole: add file lock to xenconsole
@ 2011-05-24 14:13 Yufang Zhang
2011-05-24 17:09 ` Ian Jackson
0 siblings, 1 reply; 4+ messages in thread
From: Yufang Zhang @ 2011-05-24 14:13 UTC (permalink / raw)
To: xen-devel; +Cc: pbonzini, yuzhang, yufang521247, stefano.stabellini
This patch add a file lock to xenconsole for each console id, so
that only one console could be attached to a guest at a time.
Otherwise, consoles would get stuck and print strange outputs.
Signed-off-by: Yufang Zhang <yuzhang@redhat.com>
---
tools/console/client/main.c | 31 +++++++++++++++++++++++++++++++
1 files changed, 31 insertions(+), 0 deletions(-)
diff --git a/tools/console/client/main.c b/tools/console/client/main.c
index df3636f..d7461a8 100644
--- a/tools/console/client/main.c
+++ b/tools/console/client/main.c
@@ -46,6 +46,28 @@
static volatile sig_atomic_t received_signal = 0;
+static int console_locked(const char *file)
+{
+ int fd;
+
+ fd = open(file, O_RDWR | O_CREAT, S_IRUSR|S_IWUSR);
+ if (fd == -1) {
+ fprintf(stderr,"can't open lock file: %s\n", file);
+ exit(1);
+ }
+
+ if (lockf(fd, F_TLOCK, 0) == -1) {
+ if ( errno == EACCES || errno == EAGAIN ) {
+ close(fd);
+ return (1);
+ }
+ fprintf(stderr,"can't lock file: %s\n", file);
+ exit(1);
+ }
+
+ return (0);
+}
+
static void sighandler(int signum)
{
received_signal = 1;
@@ -280,6 +302,7 @@ int main(int argc, char **argv)
int spty, xsfd;
struct xs_handle *xs;
char *end;
+ char buf[100];
console_type type = CONSOLE_INVAL;
while((ch = getopt_long(argc, argv, sopt, lopt, &opt_ind)) != -1) {
@@ -368,6 +391,13 @@ int main(int argc, char **argv)
exit(EINVAL);
}
+ /* Make sure only one console is attached to guest */
+ sprintf(buf, "/tmp/xenconsole-%d-%d", domid, num);
+ if (console_locked(buf) == 1) {
+ fprintf(stderr, "Another console has already been attached to guest\n");
+ exit(EINVAL);
+ }
+
/* Set a watch on this domain's console pty */
if (!xs_watch(xs, path, ""))
err(errno, "Can't set watch for console pty");
@@ -387,6 +417,7 @@ int main(int argc, char **argv)
console_loop(spty, xs, path);
restore_term(STDIN_FILENO, &attr);
+ unlink(buf);
free(path);
free(dom_path);
return 0;
--
1.7.4.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] xenconsole: add file lock to xenconsole
2011-05-24 14:13 [PATCH] xenconsole: add file lock to xenconsole Yufang Zhang
@ 2011-05-24 17:09 ` Ian Jackson
2011-05-27 3:20 ` Yufang Zhang
0 siblings, 1 reply; 4+ messages in thread
From: Ian Jackson @ 2011-05-24 17:09 UTC (permalink / raw)
To: Yufang Zhang; +Cc: pbonzini, xen-devel, yufang521247, stefano.stabellini
Yufang Zhang writes ("[Xen-devel] [PATCH] xenconsole: add file lock to xenconsole"):
> This patch add a file lock to xenconsole for each console id, so
> that only one console could be attached to a guest at a time.
> Otherwise, consoles would get stuck and print strange outputs.
If only we had a better console protocol, it would be possible to
attach multiple times. Oh well. In the meantime your semantic change
is sensible.
However:
> +static int console_locked(const char *file)
> +{
> + int fd;
> +
You need to use the same indent level and coding style as the
surrounding code.
> + sprintf(buf, "/tmp/xenconsole-%d-%d", domid, num);
The lockfile should be in /var/run/xen. You should use snprintf.
What arrangements do you plan to make for cleaning up stale lockfiles
(which I think will be left behind if the xenconsole client crashes) ?
Are we just going to rely on them being removed at reboot and apart
from that let them accumulate a bit ?
Ian.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] xenconsole: add file lock to xenconsole
2011-05-24 17:09 ` Ian Jackson
@ 2011-05-27 3:20 ` Yufang Zhang
2011-05-27 8:09 ` Ian Campbell
0 siblings, 1 reply; 4+ messages in thread
From: Yufang Zhang @ 2011-05-27 3:20 UTC (permalink / raw)
To: Ian Jackson; +Cc: pbonzini, Yufang Zhang, xen-devel, stefano.stabellini
On 05/25/2011 01:09 AM, Ian Jackson wrote:
> Yufang Zhang writes ("[Xen-devel] [PATCH] xenconsole: add file lock to xenconsole"):
>> This patch add a file lock to xenconsole for each console id, so
>> that only one console could be attached to a guest at a time.
>> Otherwise, consoles would get stuck and print strange outputs.
> If only we had a better console protocol, it would be possible to
> attach multiple times. Oh well. In the meantime your semantic change
> is sensible.
>
> However:
>
>> +static int console_locked(const char *file)
>> +{
>> + int fd;
>> +
> You need to use the same indent level and coding style as the
> surrounding code.
>
>> + sprintf(buf, "/tmp/xenconsole-%d-%d", domid, num);
> The lockfile should be in /var/run/xen. You should use snprintf.
>
Thanks Ian. Version2 patch has been sent, please review.
> What arrangements do you plan to make for cleaning up stale lockfiles
> (which I think will be left behind if the xenconsole client crashes) ?
> Are we just going to rely on them being removed at reboot and apart
> from that let them accumulate a bit ?
>
> Ian.
Considering the lock files are just blank and can be reused, leaving
them behind is not that bad?
Yufang
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] xenconsole: add file lock to xenconsole
2011-05-27 3:20 ` Yufang Zhang
@ 2011-05-27 8:09 ` Ian Campbell
0 siblings, 0 replies; 4+ messages in thread
From: Ian Campbell @ 2011-05-27 8:09 UTC (permalink / raw)
To: Yufang Zhang
Cc: pbonzini, Yufang Zhang, Ian Jackson, xen-devel, Stefano Stabellini
On Fri, 2011-05-27 at 04:20 +0100, Yufang Zhang wrote:
> On 05/25/2011 01:09 AM, Ian Jackson wrote:
> > Yufang Zhang writes ("[Xen-devel] [PATCH] xenconsole: add file lock to xenconsole"):
> >> This patch add a file lock to xenconsole for each console id, so
> >> that only one console could be attached to a guest at a time.
> >> Otherwise, consoles would get stuck and print strange outputs.
> > If only we had a better console protocol, it would be possible to
> > attach multiple times. Oh well. In the meantime your semantic change
> > is sensible.
> >
> > However:
> >
> >> +static int console_locked(const char *file)
> >> +{
> >> + int fd;
> >> +
> > You need to use the same indent level and coding style as the
> > surrounding code.
> >
> >> + sprintf(buf, "/tmp/xenconsole-%d-%d", domid, num);
> > The lockfile should be in /var/run/xen. You should use snprintf.
> >
>
> Thanks Ian. Version2 patch has been sent, please review.
>
> > What arrangements do you plan to make for cleaning up stale lockfiles
> > (which I think will be left behind if the xenconsole client crashes) ?
> > Are we just going to rely on them being removed at reboot and apart
> > from that let them accumulate a bit ?
> >
> Considering the lock files are just blank and can be reused, leaving
> them behind is not that bad?
Well, even an empty file has a (small) overhead. It'd be nice to avoid
if we can.
Is it permissible to use lockf on a tty device directly?
http://pubs.opengroup.org/onlinepubs/009695399/functions/lockf.html says
Record locking with lockf() shall be supported for regular files
and may be supported for other files.
so I guess it's a question of what the OSes we support actually
implement.
Ian.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-05-27 8:09 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-24 14:13 [PATCH] xenconsole: add file lock to xenconsole Yufang Zhang
2011-05-24 17:09 ` Ian Jackson
2011-05-27 3:20 ` Yufang Zhang
2011-05-27 8:09 ` Ian Campbell
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.