[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

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.

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.




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