On Fri, Apr 7, 2023 at 9:44 AM Dave Voutila <d...@sisu.io> wrote:
...
> Touch longer, but won't generate ktrace noise by blind calls to close(2)
> and also accounts for the other error conditions (EINTR, EIO).
>
> For EIO, not sure yet how we want to handle it other than log it.
>
> For EINTR, we want to account for that race and make sure we retry since
> the vmm process is long-lived and could inadvertently keep things like
> tty fds or disk image fds open after the guest vm terminates.
>
So, this is an area where
* the current POSIX standard leaves the behavior unspecified
* everyone (except HP-UX) made close() *always* invalidate the fd, even if
it fails with EINTR
* the next version of POSIX requires close() to *not* invalidate the fd if
it fails with EINTR:
* in those cases where something was interrupted but close()
invalidates the fd and previously return EINTR
they are supposed to now return EINPROGRESS
* also, a new posix_close() interface and POSIX_CLOSE_RESTART #define,
yadda yadda,
* OpenBSD has not changed to that behavior or added those interfaces at
this time, AND
* I'm not sure vmd can even hit this issue due to what it'll be closing!
Last things first: when does close() return EINTR? My understanding is
that the only case that happens in OpenBSD is interrupt of a FINAL close
(no other dup()s open, etc) of a file on NFS with unflushed writes. I
think it could happen on other systems on interrupt of close of a
rewinding-tape devices (e.g. /dev/rst0) but I don't actually see how that
would happen in OpenBSD. Are any of the fds in this case subject to those
sorts of concerns? If they're all either duplicates in a forked process,
or file types that can't block in close like that (say, sockets and
/dev/vmd devices or such), then this is all ignorable.
Meanwhile, the loop you're writing is correct for a future that OpenBSD has
not embraced. You could kind of future-proof it by making the loop
conditional on POSIX_CLOSE_RESTART being defined: if it's defined then
presumably close() has the new POSIX behavior, if not (the current state!)
then the loop is useless or actively dangerous, depending on whether the
process is multi-threaded.
I suspect at some point we'll:
* map EINTR to EINPROGRESS in sys_close()
* #define POSIX_CLOSE_RESTART 0 /* explicitly blessed by POSIX */
* add to libc int posix_close(int fd, int flags) { int ret = close(fd);
return flags ? EINVAL : ret; }
'cause it'll mean less patching for ports, but someone will need to check
for anything in tree that tries to be clever about close() returning EINTR.
Overall , a great use of everyone's time. /s
Philip
