qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Wouter Verhelst <w@uter.be>
To: Eric Blake <eblake@redhat.com>
Cc: nbd list <nbd@other.debian.org>,
	"Qemu-devel@nongnu.org" <Qemu-devel@nongnu.org>,
	qemu block <qemu-block@nongnu.org>
Subject: Re: [Qemu-devel] RFC: Let NBD client request read-only mode
Date: Thu, 30 Nov 2017 16:32:13 +0100	[thread overview]
Message-ID: <20171130153213.GB11253@grep.be> (raw)
In-Reply-To: <15f2fce6-00f4-0c1d-4ebe-d4ed7c5e7596@redhat.com>

On Wed, Nov 29, 2017 at 08:57:20AM -0600, Eric Blake wrote:
> Right now, only the server can choose whether an export is read-only.  A
> client can always treat an export as read-only by not sending any writes,
> but a server has no guarantee that a client will behave that way, and must
> assume that an export where the server did not advertise NBD_FLAG_READ_ONLY
> will modify the export.  Therefore, if the server does not want to permit
> simultaneous modifications to the underlying data, it has the choice of
> either permitting only one client at a time, or supporting multiple
> connections but enforcing all subsequent connections to see the
> NBD_FLAG_READ_ONLY bit on the export that is already in use by the first
> connection (note that this is racy - whoever connects first is the only one
> that can get write permissions, even if the first connected client doesn't
> want to write).
> 
> However, at least qemu has a case where it would be nice to permit a
> parallel known-read-only client from the same server that is (or will be)
> handling a read-write client; and what's more, to make it so that the
> read-only client can win the race of being the first connection without
> penalizing the actual read-write connection (see
> https://bugzilla.redhat.com/show_bug.cgi?id=1518543).

Right, I can see the dilemma.

(a possible workaround could be that the server could have two versions of the
same export, one which is marked read-only and one which is not, but that is a
bit ugly)

> I don't see any way to accomplish this with oldstyle negotiation (but that
> doesn't matter these days); but with newstyle negotiation, there are at least
> two possible implementations:
> 
> Idea 1: the server advertises a new global bit NBD_FLAG_NO_WRITE (ideas for
> a better name?) in its 16-bit handshake flags; if the client replies with
> the same bit set (documentation-wise, we'd name the client reply
> NBD_FLAG_C_NO_WRITE), then the server knows that the client promises to be a
> read-only connection.

I'd rather not burn a global bit for this.

> Idea 2: we add a new option, NBD_OPT_READ_ONLY.  If the client sends this
> option, and the server replies with NBD_REP_ACK, then the server knows that
> the client promises to be a read-only connection.
> 
> With either idea, once the server knows the client's intent to be a
> read-only client, the server SHOULD set NBD_FLAG_READ_ONLY on all (further)
> information sent for any export (whether from NBD_OPT_EXPORT_NAME,
> NBD_OPT_INFO, or NBD_OPT_GO) and treat any export as read-only for the
> current client, even if that export is in parallel use by another read-write
> client, and the client MUST NOT send NBD_CMD_WRITE, NBD_CMD_TRIM,
> NBD_CMD_WRITE_ZEROES, or any other command that requires a writable
> connection (the NBD_CMD_RESIZE extension comes to mind).

Right.

> A client that wants to be read-only, but which does not see server support
> (in idea 1, the server did not advertise the bit; in idea 2, the server
> replies with NBD_REP_ERR_UNSUP), does not have to do anything special (it is
> always possible to do just reads to a read-write connection, and the server
> may still set NBD_FLAG_READ_ONLY even without supporting the extension of
> permitting a client-side request).  But such a client may, if it wants to be
> nice to potential parallel writers on the same export, decide to disconnect
> quickly (with NBD_OPT_ABORT or NBD_CMD_DISC as appropriate) rather than tie
> up a read-write connection.

Indeed.

> I don't know which idea is more palatable.  We have a finite set of only 2^4
> global handshake flags because it is a bitmask, where only 14 bits remain;
> whereas we have almost 2^32 potential NBD_OPT_ values.  On the other hand,
> using a global handshake flag means the server never shows any export as
> writable; while with the NBD_OPT_ solution, a guest can get different
> results for the sequence NBD_OPT_INFO, NBD_OPT_READ_ONLY, NBD_OPT_INFO.

It might additionally also be a good idea to add another data item to
the NBD_OPT_INFO response which tells the client that it will be the
only writer, but that there may be other readers.

That way, if a client sees that data item, it could go "oh, but I don't
need to write -- here's an NBD_OPT_READ_ONLY for you".

> There's also the question with option 2 of whether permitting
> NBD_OPT_READ_ONLY prior to NBD_OPT_STARTTLS would make sense (is there any
> case where the set of TLS authentication to be performed can involve looser
> requirements for a known-read-only client?),

Yes, but if a server wants to allow writing to a device based on whether
a client is authenticated or not, then all it needs to do is to set the
read-only flag based on whether that client is authenticated or not.

It might still be useful to signal to a client somehow that it could get
more rights if it provided authentication credentials of some sort, but
that is not entirely related to whether or not a client declares that it
will write to the device

> where using a global bit makes the sequence of required NBD_OPT_* a
> bit less stateful.
> 
> Does the idea sound reasonable enough to propose wording to add it to the
> NBD spec and an implementation in qemu?  Which of the two ideas is preferred
> for letting the client inform the server of its intent?

I think it sounds reasonable enough, yes; but I also think there are a
few other related situations that might be relevant enough to warrant
thinking about more. I gave a few examples above, but maybe there are
more? Dunno.

-- 
Could you people please use IRC like normal people?!?

  -- Amaya Rodrigo Sastre, trying to quiet down the buzz in the DebConf 2008
     Hacklab

  reply	other threads:[~2017-11-30 15:33 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-29 14:57 [Qemu-devel] RFC: Let NBD client request read-only mode Eric Blake
2017-11-30 15:32 ` Wouter Verhelst [this message]
2017-11-30 16:00   ` Eric Blake
2017-11-30 17:43     ` Wouter Verhelst

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171130153213.GB11253@grep.be \
    --to=w@uter.be \
    --cc=Qemu-devel@nongnu.org \
    --cc=eblake@redhat.com \
    --cc=nbd@other.debian.org \
    --cc=qemu-block@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).