qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Eric Blake <eblake@redhat.com>
Cc: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	nbd-general@lists.sourceforge.net, qemu-devel@nongnu.org,
	Pavel Borzenkov <pborzenkov@virtuozzo.com>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	Wouter Verhelst <w@uter.be>, "Denis V. Lunev" <den@openvz.org>
Subject: Re: [Qemu-devel] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension
Date: Fri, 8 Apr 2016 13:35:08 +0200	[thread overview]
Message-ID: <20160408113508.GE4700@noname.redhat.com> (raw)
In-Reply-To: <57068692.2080208@redhat.com>

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

Am 07.04.2016 um 18:10 hat Eric Blake geschrieben:
> On 04/07/2016 04:38 AM, Vladimir Sementsov-Ogievskiy wrote:
> > On 05.04.2016 16:43, Paolo Bonzini wrote:
> >>
> >> On 05/04/2016 06:05, Kevin Wolf wrote:
> >>> The options I can think of is adding a request field "max number of
> >>> descriptors" or a flag "only single descriptor" (with the assumption
> >>> that clients always want one or unlimited), but maybe you have a better
> >>> idea.
> >> I think a limit is better.  Even if the client is ultimately going to
> >> process the whole file, it may take a very long time and space to
> >> retrieve all the descriptors in one go.  Rather than query e.g. 16GB at
> >> a time, I think it's simpler to put a limit of 1024 descriptors or so.
> >>
> >> Paolo
> >>
> > 
> > I vote for the limit too. More over, I think, there should be two sides
> > limit:
> > 
> > 1. The client can specify the limit, so server should not return more
> > extents than requested. Of course, server should chose sequential
> > extents from the beginning of requested range.
> 
> For the client to request a limit would entail that we enhance the
> protocol to allow structured requests (where a wire-sniffer would know
> how many bytes to read for the client's additional data, even if it does
> not understand the extension's semantics).  Might not be a bad idea to
> have this in the long run, but so far I've been reluctant to bite the
> bullet.

I still think that client specified limits are the more important part
because only the client knows what information it needs. Server limits
can protect the server against excessive requests from clients, but only
the client can protect against wasting resources by doing more than is
necessary.

> > 2. Server side limit: if client asked too many extents or not specified
> > a limit at all, server should not return all extents, but only 1024 (for
> > ex.) from the beginning of the range.
> 
> Okay, I'm fairly convinced now that letting the server limit the reply
> is a good thing, and that one doesn't require a structured request from
> the client.  Since we just recently documented that strings should be no
> more than 4096 bytes, and my v2 proposal used 8 bytes per descriptor,
> maybe a good way to enforce a similar limit would be:
> 
> The server MAY choose to send fewer descriptors than what would describe
> the full extent of the client's request, but MUST send at least one
> descriptor unless an error is reported.  The server MUST NOT send more
> than 512 descriptors, even if that does not completely describe the
> client's requested length.

Why have these 512 descriptors in the spec? Sounds to me as if it should
be a server configuration option.

> That way, a client in general should never expect more than ~4096 bytes
> + overhead on any server reply except a reply to NBD_CMD_READ, and can
> therefore utilize stack allocation for all other replies (if we do this,
> maybe we should make a hard rule that all future protocol extensions,
> other than NBD_CMD_READ, will guarantee that a reply has a bounded size)

Or have a client specified limit for all potentially unbounded replies.

> I also think it may be okay to let the server reply with MORE data than
> the client requested, but only as long as it does not result in any
> extra descriptors (that is, only the last descriptor can result in a
> length beyond the client's request).  For example, if the client asks
> for block status of 1M of the file, but the server can conveniently
> learn via lseek(SEEK_HOLE) or other means that there are 2M of data
> before status changes, then there's no reason to force the server to
> throw away the information about the 1M beyond the client's read, and
> the client might even be able to be more efficient in later requests.

If we say SHOULD instead of MAY, this might actually be a way around the
urgent practical problem that qemu needs a way to ask the question
"what's the status of this block and how many contiguous blocks have the
same status? (I don't care about the following extents.)" This is
actually the only way that qemu is currently planning to use the
command.

So if qemu can ask for "status at offset x, length 1 byte" and expect to
get the right reply to the above question, that might be workable.

Kevin

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

  parent reply	other threads:[~2016-04-08 11:35 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-04 16:39 [Qemu-devel] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension Eric Blake
2016-04-04 18:06 ` [Qemu-devel] [Nbd] " Alex Bligh
2016-04-04 19:34   ` Eric Blake
2016-04-04 19:54     ` Denis V. Lunev
2016-04-04 20:03       ` Alex Bligh
2016-04-04 20:08         ` Denis V. Lunev
2016-04-04 20:34           ` Eric Blake
2016-04-04 21:06             ` Denis V. Lunev
2016-04-04 21:12             ` Alex Bligh
2016-04-05 14:15         ` Paolo Bonzini
2016-04-05 15:01           ` Alex Bligh
2016-04-05 15:23             ` Paolo Bonzini
2016-04-05 15:27               ` Alex Bligh
2016-04-05 15:31                 ` Paolo Bonzini
2016-04-04 23:08       ` Wouter Verhelst
2016-04-04 23:32         ` Eric Blake
2016-04-05  7:16           ` Wouter Verhelst
2016-04-05 21:44           ` Wouter Verhelst
2016-04-05  7:13         ` Alex Bligh
2016-04-04 19:58     ` Alex Bligh
2016-04-04 20:04       ` Denis V. Lunev
2016-04-04 20:08         ` Alex Bligh
2016-04-04 20:13           ` Denis V. Lunev
2016-04-04 20:15             ` Alex Bligh
2016-04-04 20:27               ` Denis V. Lunev
2016-04-04 20:45                 ` Eric Blake
2016-04-04 21:04                   ` Denis V. Lunev
2016-04-04 21:12                     ` Alex Bligh
2016-04-04 21:17                     ` Eric Blake
2016-04-04 21:27                       ` Denis V. Lunev
2016-04-04 20:26           ` Eric Blake
2016-04-04 21:07             ` Alex Bligh
2016-04-04 21:25               ` Eric Blake
2016-04-04 22:06                 ` Alex Bligh
2016-04-04 20:22       ` Eric Blake
2016-04-05 13:38     ` Paolo Bonzini
2016-04-04 22:40 ` Wouter Verhelst
2016-04-04 23:03   ` Eric Blake
2016-04-05 13:41     ` Paolo Bonzini
2016-04-06  5:57     ` Denis V. Lunev
2016-04-06 14:08       ` Eric Blake
2016-04-05  4:05 ` [Qemu-devel] " Kevin Wolf
2016-04-05 13:43   ` Paolo Bonzini
2016-04-07 10:38     ` Vladimir Sementsov-Ogievskiy
2016-04-07 16:10       ` Eric Blake
2016-04-07 16:21         ` [Qemu-devel] [Nbd] " Alex Bligh
2016-04-08 11:35         ` Kevin Wolf [this message]
2016-04-09  9:08         ` Wouter Verhelst
2016-04-13 12:38         ` [Qemu-devel] " Pavel Borzenkov
2016-04-13 14:40           ` Eric Blake
2016-04-07 15:35     ` Pavel Borzenkov
2016-04-07 15:43       ` Paolo Bonzini
2016-04-05  8:51 ` Stefan Hajnoczi
2016-04-05  9:24 ` [Qemu-devel] [Nbd] " Markus Pargmann
2016-04-05 13:50   ` Paolo Bonzini
2016-04-11  5:58     ` Markus Pargmann
2016-04-05 14:14   ` Eric Blake
2016-04-05 20:50     ` Wouter Verhelst
2016-04-11  6:07       ` Markus Pargmann

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=20160408113508.GE4700@noname.redhat.com \
    --to=kwolf@redhat.com \
    --cc=den@openvz.org \
    --cc=eblake@redhat.com \
    --cc=nbd-general@lists.sourceforge.net \
    --cc=pbonzini@redhat.com \
    --cc=pborzenkov@virtuozzo.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    --cc=vsementsov@virtuozzo.com \
    --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).