[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 05/24] Introduce locking functions for block device setup on NetBSD

On Wed, Jan 20, 2021 at 03:13:22PM +0000, Ian Jackson wrote:
> Manuel Bouyer writes ("Re: [PATCH 05/24] Introduce locking functions for 
> block device setup on NetBSD"):
> > On Tue, Dec 29, 2020 at 12:29:09PM +0100, Roger Pau Monné wrote:
> > > I think you want tot CC the tools dev on this one, specially Ian who
> > > knows how the Linux one is implemented and can likely give valuable
> > > input.
> ...
> > > Seeing the file itself, I don't think there's any NetBSD specific
> > > stuff, so we might want to consider putting it in BSD/ instead, so it
> > > can be used by FreeBSD also?
> > 
> > I'm not sure if FreeBSD needs the locking stuff.
> > Also, there are certainly differences in block device handling between
> > FreeBSD and NetBSD. Both OSes have diverged in this area.
> I think most operating systems will want some kind of locking here.
> I loooked at the code in the new tools/hotplug/NetBSD/locking.sh.
> Unfortunately this area is complex and the available APIs and tools
> are awkard, and the field is troubled by broken "traditional"
> approaches involving O_EXCL or the moral equivalent, which cannot be
> made reliable (if you think reliability implies never being broken due
> to stale lock).
> I doubt that the code in this patch is correct.  It uses shlock(1)
> which is based on link(2) and kill(2) and so on, which I think is
> basically an O_EXCL-based approach as I discuss above.  (I don't have
> a formal proof of this contention.)  The presence of an invocation of
> the "trap" shell builtin in the new NetBSD script is a bad sign - a
> reliable locking protocol would need that.

Actually this patch is old - since Xen 4.8 at last.

> I see from https://man.netbsd.org that NetBSD has flock(1) and
> stat(1).  I think this means we could reuse the code in
> tools/hotplug/Linux/locking.sh.  Maybe it will need to be lightly
> adapted, to NetBSD's flock(1) and stat(1).  Perhaps via some kind of
> substitution to avoid all the clone-and-hack.

Yes, at last the stat call will need to be patched.
But it seems to rely on a linux-specific behavoir, which is that
/dev/stdin points to the real file on redirection:
>ls -l /dev/stdin /proc/self/fd/0 < /etc/passwd
lrwxrwxrwx 1 root   root      15 Apr 30  2019 /dev/stdin -> /proc/self/fd/0
lr-x------ 1 bouyer ita-iatos 64 Jan 20 16:54 /proc/self/fd/0 -> /etc/passwd

On NetBSD (and I guess other BSDs) this won't work, as /dev/stdin is a
specific device:
>ls -l /dev/stdin 
crw-rw-rw-  1 root  wheel  22, 0 Nov 15  2007 /dev/stdin

so stat -L will always return the same data. We can't use the same protocol.

Manuel Bouyer <bouyer@xxxxxxxxxxxxxxx>
     NetBSD: 26 ans d'experience feront toujours la difference



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.