Re: [Xen-devel] [PATCH] xenconsole: add file lock to xenconsole

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

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.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel