qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Wouter Verhelst <w@uter.be>, Alex Bligh <alex@alex.org.uk>
Cc: "nbd-general@lists.sourceforge.net"
	<nbd-general@lists.sourceforge.net>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>
Subject: [Qemu-devel] NBD_REP_SERVER layout [was: [PATCHv2] Strawman proposal for NBD structured replies]
Date: Wed, 30 Mar 2016 13:10:29 -0600	[thread overview]
Message-ID: <56FC24A5.1090209@redhat.com> (raw)
In-Reply-To: <56FC1E6D.8060104@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 2987 bytes --]

On 03/30/2016 12:43 PM, Eric Blake wrote:

> On that tangent, I found SELECT slightly ambiguous (particularly since
> I'm also considering a proposal to modify NBD_REP_SERVER to expose
> alignment details, so it would have to play nicely with SELECT):
> 
> Based on normative text alone, we would have:
> 
> S: 64 bits, 0x3e889045565a9
> S: 32 bits, NBD_OPT_SELECT (6)
> S: 32 bits, NBD_REP_SERVER (2)
> S: 32 bits, length
> S: 32 bits, name length
> S: 'name length' bytes, UTF-8 encoded name (is it NUL-terminated?)
> S: 'length - name length' bytes, which is UTF-8 encoded message to
> display to human
> 
> This implies that 'name length' <= 'length - 4', although we don't
> actually state that the server MUST NOT send a name length longer than
> that.  It might not hurt to also require that length include space for a
> NUL terminator (in both the name, and in the optional human-readable
> information field), but only if that matches existing practice (if it
> does not, then we should document that the client and server are NOT
> dealing with NUL-terminated UTF-8 strings, but relying on length instead).
> 
> The SELECT text then refines this:
> 
> S: 64 bits, 0x3e889045565a9
> S: 32 bits, NBD_OPT_SELECT (6)
> S: 32 bits, NBD_REP_SERVER (2)
> S: 32 bits, length
> S: 32 bits, name length
> S: 'name length' bytes, UTF-8 encoded name (is it NUL-terminated?)
> S: 64 bits, size
> S: 16 bits, export flags
> 
> but doesn't mention what happens if 'length' > 'name length + 14'.
> Presumably, if the server wants to include the same UTF-8 human
> information as before, it would go AFTER the 16 bits for the export
> field (in other words, the SELECT extension is carving out fields in the
> MIDDLE of the payload).

Actually, now that I've thought about it, I wonder if it is better to
arrange data so that there is _always_ a UTF-8 human-readable string
immediately after the name (with 1-byte content of "" as the minimum),
so that clients can just blindly print that string, even if binary data
appears later in the structure due to other things (such as
NBD_OPT_SELECT) specifying the layout of that binary data.  But if we go
with that approach, then we should probably document that NBD_REP_SERVER
sent in reply to NBD_OPT_SELECT must have length >= 'name-length + 15'.
 Anywhere that we put binary data where a client used to be expecting
human-readable data risks an unprepared client printing garbage.

> 
> So, once I know for sure about the handling of NUL bytes, I'll probably
> try my hand at a patch to clarify the wording in both the normative and
> in the SELECT extension.

At any rate, I hope that I've brought attention to the issue, and that
part of moving SELECT out of experimental and into normative will
involve documenting decisions you actually make while implementing things.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

  reply	other threads:[~2016-03-30 19:10 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-29 22:48 [Qemu-devel] [PATCHv2] Strawman proposal for NBD structured replies Alex Bligh
2016-03-29 23:17 ` Eric Blake
2016-03-30  6:59   ` Alex Bligh
2016-03-30  7:33     ` Wouter Verhelst
2016-03-30 18:43       ` Eric Blake
2016-03-30 19:10         ` Eric Blake [this message]
2016-03-30 20:12         ` [Qemu-devel] [Nbd] " 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=56FC24A5.1090209@redhat.com \
    --to=eblake@redhat.com \
    --cc=alex@alex.org.uk \
    --cc=nbd-general@lists.sourceforge.net \
    --cc=qemu-devel@nongnu.org \
    --cc=w@uter.be \
    /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).