Josef,
> On 3 Oct 2016, at 15:32, Josef Bacik <jba...@fb.com> wrote:
>
> Ok I understand your objections now. You aren't arguing that we are unsafe
> by default, only that we are unsafe with servers that do something special
> beyond simply writing to a single disk or file. I agree this is problematic,
> but you simply don't use this feature if your server can't deal with it well.
Not quite. I am arguing that:
1. Parallel channels as currently implemented are inherently unsafe on some
servers - I have given an example of such servers
2. Parallel channels as currently implemented may be unsafe on the reference
server on some or all platforms (depending on the behaviour of fdatasync()
which might varies between platforms)
3. Either the parallel channels stuff should issue flush requests on all
channels, or the protocol should protect the unwitting user by negotiating an
option to do so (or refusing multi-channel connects). It is not reasonable for
an nbd client to have to know the intimate details of the server and its
implementation of synchronisation primitives - saying 'the user should disable
multiple channels' is not good enough, as this is what we have a protocol for.
"unsafe" here means 'do not conform to the kernel's requirements for flush
semantics, whereas they did without multiple channels'
So from my point of view either we need to have an additional connection flag
("don't use multiple channels unless you are going to issue a flush on all
channels") OR flush on all channels should be the default.
Whatever SOME more work needs to be done on your patch because there it current
doesn't issue flush on all channels, and it currently does not have any way to
prevent use of multiple channels.
I'd really like someone with linux / POSIX knowledge to confirm the linux and
POSIX position on fdatasync of an fd opened by one process on writes made to
the same file by another process. If on all POSIX platforms and all filing
systems this is guaranteed to flush the second platform's writes, then I think
we could argue "OK there may be some weird servers which might not support
multiple channels and they just need a way of signalling that". If on the other
hand there is no such cross platform guarantee, I think this means in essence
even with the reference server, this patch is unsafe, and it needs adapting to
send flushes on all channels - yes it might theoretically be possible to
introduce IPC to the current server, but you'd still need some way of tying
together channels from a single client.
--
Alex Bligh
