[Libguestfs] [PATCH nbdkit 0/7] server: Implement NBD_FLAG_CAN_MULTI_CONN.

[Richard W.M. Jones]

On Fri, Jan 04, 2019 at 05:26:07PM -0600, Eric Blake wrote:
> On 1/4/19 4:08 PM, Richard W.M. Jones wrote:
> > First thing to say is that I need to do a *lot* more testing on this,
> > so this is just an early peek.  In particular, although it passed
> > ‘make check && make check-valgrind’ I have *not* tested it against a
> > multi-conn-aware client such as the Linux kernel >= 4.9.
> > 
> > This implements NBD_FLAG_CAN_MULTI_CONN, described in the protocol doc
> > as:
> > 
> >   "NBD_FLAG_CAN_MULTI_CONN: Indicates that the server operates
> >   entirely without cache, or that the cache it uses is shared among
> >   all connections to the given device. In particular, if this flag is
> >   present, then the effects of NBD_CMD_FLUSH and NBD_CMD_FLAG_FUA MUST
> >   be visible across all connections when the server sends its reply to
> >   that command to the client. In the absence of this flag, clients
> >   SHOULD NOT multiplex their commands over more than one connection to
> >   the export."
> > 
> > This is necessary to support the Linux nbd client -C option.
> 
> Sounds promising. And reminds me that I have not even tried to implement
> this option for qemu yet, but probably should.

I've yet to do testing, but it seems to be necessary if we're going to
do scaling with the Linux client.

> > The only plugin which sets the flag so far is file.  The ocaml, sh and
> > nbd plugins allow the flag to be controlled or passed through.
> > 
> > Notable also is that the blocksize filter has to filter out this flag,
> > because I'm not convinced that the bounce buffer is safe.  However I
> > believe the other filters *are* safe (although not really certain
> > about the fua filter, and the new cache filter is tricky too).
> > 
> > My feeling is that we should set the flag unconditionally for all
> > readonly connections, but I can think of nasty corner cases where it
> > might not work.  We should most probably set it in all plugins that
> > are readonly (eg. nbdkit-pattern-plugin).
> 
> Should the callback include a bool readonly parameter that lets the
> filter/plugin know for sure whether it is answering the question on a
> read-only connection vs. on a read-write connection, as the answer may
> differ between those two?

Yes, I think this is a good idea.  Will try that for v2.

> Should we automatically set the bit for any plugin that has
> fully-serialized operation but does not provide the callback?  That is,
> if a plugin uses SERIALIZE_CONNECTIONS, you can never have multiple
> clients at the same time anyways (so the flag is trivially ignorable - a
> client can try to set up a second connection for speed, but it won't
> help).  A plugin that uses SERIALIZE_ALL_REQUESTS is also trivially
> supported - any flush request will complete (regardless of which
> connected client made it) prior to any other client being able to make
> progress on a request that would need to see the results of the flush.
> Only when we get to SERIALIZE_REQUESTS or PARALLEL can we have the
> situation where one client's request can outpace the flush operation in
> another client.

I believe the answer here is _no_:

Imagine a plugin which serves different data on different connections,
ie. complete different data, such as a "roulette" plugin which selects
a different disk for each handle.  Such a plugin is of course insane,
and it's not something that any of _our_ plugins do, but it's
something which is possible.

Currently a roulette plugin works with well-behaved clients because
they will only open a single connection and thus will always see a
single view of the virtual disk.  However if this plugin advertized
NBD_FLAG_CAN_MULTI_CONN then clients could connect multiple times, and
would see completely different data on each connection, which would
break those clients horribly.

Note this argument is not affected by the thread model or the readonly
flag.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top