necting to shonkynbd', a message in
the commit log might be useful.
Otherwise:
Reviewed-by: Alex Bligh
> ---
> include/block/nbd.h | 1 +
> nbd/nbd-internal.h | 7 +
> nbd/client.c| 86 +++--
> 3 files changed, 92 in
returns the same information but does not
> transition to transmission phase.
>
> Signed-off-by: Eric Blake
Reviewed-by: Alex Bligh
> ---
> nbd/server.c | 122 ---
> 1 file changed, 109 insertions(+), 13 deletions(-)
>
t; -errno = 0;
> +goto out;
> +case NBD_CMD_CLOSE:
> +TRACE("Request type is CLOSE");
> +if (request.flags || request.from || request.len) {
> +LOG("bad parameters, skipping flush");
> +reply.error = EINVAL;
> +} else {
> +ret = blk_co_flush(exp->blk);
> +if (ret < 0) {
> +LOG("flush failed");
> +reply.error = -ret;
> +}
> +}
> +/* Attempt to send reply, but even if it fails, we are done */
> +nbd_co_send_reply(req, &reply, 0);
> goto out;
> case NBD_CMD_FLUSH:
> TRACE("Request type is FLUSH");
> --
> 2.5.5
>
>
--
Alex Bligh
0) {
> +LOG("writing to file failed");
> +reply.error = -ret;
> +goto error_reply;
> +}
> +
> +/* FIXME: do we need FUA flush here, if we also passed it to
> + * blk_write_zeroes? */
I don't think so. blk_write_zeroes with BDRV_REQ_FUA should
make *that* request access the media, but FLUSH makes ALL PREVIOUS
requests access the media, so you are doing too much work here
I think.
> +if (request.flags & NBD_CMD_FLAG_FUA) {
> +ret = blk_co_flush(exp->blk);
> +if (ret < 0) {
> +LOG("flush failed");
> +reply.error = -ret;
> +goto error_reply;
> +}
> +}
> +
> +if (nbd_co_send_reply(req, &reply, 0) < 0) {
> +goto out;
> +}
> +break;
> case NBD_CMD_DISC:
> TRACE("Request type is DISCONNECT");
> goto out;
> --
> 2.5.5
>
>
--
Alex Bligh
ritev_flags,
> .supported_write_flags = BDRV_REQ_FUA,
> @@ -516,6 +538,7 @@ static BlockDriver bdrv_nbd_unix = {
> .bdrv_parse_filename= nbd_parse_filename,
> .bdrv_file_open = nbd_open,
> .bdrv_co_readv = nbd_co_readv,
> +.bdrv_co_write_zeroes = nbd_co_write_zeroes,
> .bdrv_co_writev = nbd_co_writev,
> .bdrv_co_writev_flags = nbd_co_writev_flags,
> .supported_write_flags = BDRV_REQ_FUA,
> --
> 2.5.5
>
>
--
Alex Bligh
for a
> new protocol, so I think we should make it explicit that clients should
> not try that save in exceptional circumstances.
+1. Do you want to ping me when you have had a chance to review v5 and
I will collate all of these in to a v6?
--
Alex Bligh
Wouter,
On 9 Apr 2016, at 12:38, Wouter Verhelst wrote:
> On Sat, Apr 09, 2016 at 12:21:03PM +0100, Alex Bligh wrote:
>> An alternative route would be to delete OPTIONALTLS, and make some of
>> the MUST requirements in SELECTIVETLS say "MUST xyz unless there are
>> no
available.
Reviewed-by: Eric Blake
Signed-off-by: Alex Bligh
---
doc/proto.md | 338 +--
1 file changed, 304 insertions(+), 34 deletions(-)
Changes from v5:
* Delete OPTIONALTLS (RIP)
* Add NBD_REP_ERR_POLICY
* s/NBD_ERR_REP/NBD_REP_E
l, if FUA is still set, the server is too old, so we do
> a fallback flush to get the same semantics for the write in question,
> but at higher cost of a full flush.
OK, thanks.
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
and would be concerned that gnutls_bye()
might block.
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
available.
Signed-off-by: Alex Bligh
---
doc/proto.md | 342 +--
1 file changed, 308 insertions(+), 34 deletions(-)
Changes from v6:
* Introduced language mandating a server to reply with NBD_ERR_INVALID
to NBD_OPT_STARTTLS if TLS
gt; buggy clients.
Reinstated explicitly in the TLS section, and mentioned in the
NBD_OPT_ section too. Thanks for catching that one.
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
available.
Signed-off-by: Alex Bligh
---
doc/proto.md | 342 +--
1 file changed, 308 insertions(+), 34 deletions(-)
Changes since v7
* I missed committing the changes re consistent use of 'option' rather than
'command&#x
Wouter,
On 11 Apr 2016, at 07:10, Wouter Verhelst wrote:
> Mostly there. Final note:
>
> On Sun, Apr 10, 2016 at 01:47:32PM +0100, Alex Bligh wrote:
>> diff --git a/doc/proto.md b/doc/proto.md
>> index f117394..5005552 100644
>> --- a/doc/proto.md
>> +++ b/
Signed-off-by: Alex Bligh
---
tests/run/nbd-tester-client.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/tests/run/nbd-tester-client.c b/tests/run/nbd-tester-client.c
index 06396a7..1b99fa8 100644
--- a/tests/run/nbd-tester-client.c
+++ b/tests/run/nbd-tester
Apologies - wrong list. The perils of relying on history.
Alex
On 11 Apr 2016, at 15:31, Alex Bligh wrote:
> Signed-off-by: Alex Bligh
> ---
> tests/run/nbd-tester-client.c | 12
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/tests/run/nbd
For instance, if the client does not find an
>>> export it is looking for, it may simply send an NBD_OPT_ABORT
>>> and close the TCP connection.
>
> Otherwise, this seems reasonable, other than the fact that qemu needs
> patches to actually start sending NBD_OPT_ABORT where possible.
I'd suggest waiting for a definitive answer on whether it's meant
to have a reply.
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
s an NBD_REP_ACK from the NBD_OPT_ABORT message.
Yeah. That way would be a safe change (as the worst that can
happen is the client thinks the server has rudely dropped
the connection).
--
Alex Bligh
#x27;no way out'
situation (e.g. TLS negotiation fails), it MUST use NBD_CMD_DISC
or NBD_OPT_ABORT
* We should tidy up the semantics and descriptions of NBD_CMD_DISC
and NBD_OPT_ABORT, viz replies or not to the latter, shutting
down TLS properly etc.
--
Alex Bligh
On 12 Apr 2016, at 11:34, Daniel P. Berrange wrote:
> On Tue, Apr 12, 2016 at 10:48:20AM +0100, Daniel P. Berrange wrote:
>> On Sun, Apr 10, 2016 at 10:49:00AM +0100, Alex Bligh wrote:
>>> (Daniel: if you want to replicate the issue, just run qemu-img info
>>> agai
On 12 Apr 2016, at 13:40, Wouter Verhelst wrote:
> Right, that sounds good.
Great. I may look at that when the other doc patches are applied.
On which note, back to $subject, how is PATCHv8?
--
Alex Bligh
available.
Signed-off-by: Alex Bligh
---
doc/proto.md | 352 +--
1 file changed, 318 insertions(+), 34 deletions(-)
Changes since v8:
* Reword section on disconnection, as per mail to list save with Eric Blake's
change of &#x
on regime within
a change which is meant to be documenting TLS.
Alex
> I can probably apply it and then work on the clarification of that
> paragraph, I suppose, but I'd prefer doing it right from the get go.
--
Alex Bligh
available.
Signed-off-by: Alex Bligh
---
doc/proto.md | 352 +--
1 file changed, 318 insertions(+), 34 deletions(-)
Changes since v9
* Two further nits from Eric Blake.
Changes since v8:
* Reword section on disconnection, as per mail to
k you mean s/exports prior/responses prior/
>> +
>> +## Client-side requirements
>
> ...but this should also be H3. s/##/###/
Thanks.
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
trying, on the grounds that maybe the client will get the
>> hint to send NBD_OPT_STARTTLS.
>>
>> Signed-off-by: Eric Blake
Reviewed-by: Alex Bligh
It is worth noting that this change (assuming I've read
it right) in no way means that qemu would be serving resources
without T
Also, a (buggy/malicious) server that replies with length <
>> sizeof(namelen) would cause us to block waiting for bytes that
>> the server is not sending, and one that replies with super-huge
>> lengths could cause us to temporarily allocate up to 4G memory.
>> Sa
>
> Signed-off-by: Eric Blake
Reviewed-by: Alex Bligh
> ---
> nbd/server.c | 32 +++-
> 1 file changed, 23 insertions(+), 9 deletions(-)
>
> diff --git a/nbd/server.c b/nbd/server.c
> index fc36f4d..0a003e4 100644
> --- a/nbd/server.c
izeof(buf) - 18) !=
> @@ -806,7 +804,7 @@ static void nbd_eject_notifier(Notifier *n, void *data)
> }
>
> NBDExport *nbd_export_new(BlockBackend *blk, off_t dev_offset, off_t size,
> - uint32_t nbdflags, void (*close)(NBDExport *),
> + uint16_t nbdflags, void (*close)(NBDExport *),
> Error **errp)
> {
> NBDExport *exp = g_malloc0(sizeof(NBDExport));
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index 2c9754e..71bfdeb 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -241,7 +241,7 @@ static void *nbd_client_thread(void *arg)
> {
> char *device = arg;
> off_t size;
> -uint32_t nbdflags;
> +uint16_t nbdflags;
> QIOChannelSocket *sioc;
> int fd;
> int ret;
> @@ -455,7 +455,7 @@ int main(int argc, char **argv)
> BlockBackend *blk;
> BlockDriverState *bs;
> off_t dev_offset = 0;
> -uint32_t nbdflags = 0;
> +uint16_t nbdflags = 0;
> bool disconnect = false;
> const char *bindto = "0.0.0.0";
> const char *port = NULL;
> --
> 2.5.5
>
>
--
Alex Bligh
asier to test commit 200650d4, which
> is the client counterpart of receiving the description.
>
> Signed-off-by: Eric Blake
Reviewed-by: Alex Bligh
> ---
> include/block/nbd.h | 1 +
> nbd/nbd-internal.h | 5 +++--
> nbd/server.c| 34
goto out;
> +/* unreachable, thanks to special case in nbd_co_receive_request() */
> +abort();
> +
> case NBD_CMD_FLUSH:
> TRACE("Request type is FLUSH");
>
> @@ -1182,10 +1203,14 @@ static void nbd_trip(void *opaque)
> break;
> default:
> LOG("invalid request type (%" PRIu32 ") received", request.type);
> -invalid_request:
> reply.error = EINVAL;
> error_reply:
> -if (nbd_co_send_reply(req, &reply, 0) < 0) {
> +/* We must disconnect after replying with an error to
> + * NBD_CMD_READ, since we choose not to send bogus filler
> + * data; likewise after NBD_CMD_WRITE if we did not read the
> + * payload. */
> +if (nbd_co_send_reply(req, &reply, 0) < 0 || command == NBD_CMD_READ
> ||
> +(command == NBD_CMD_WRITE && !req->complete)) {
> goto out;
> }
> break;
> --
> 2.5.5
>
>
--
Alex Bligh
stick to the required minimum.
>
> Signed-off-by: Eric Blake
Reviewed-by: Alex Bligh
>
> ---
> v3: enlarge the limit, and document choice of new value
> ---
> include/block/nbd.h | 6 ++
> nbd/client.c| 2 +-
> nbd/server.c| 4 ++--
> 3 files changed, 9 i
ent upstream
> NBD protocol (https://github.com/yoe/nbd/blob/master/doc/proto.md),
> and touch some nearby code to keep checkpatch.pl happy.
>
> Signed-off-by: Eric Blake
Reviewed-by: Alex Bligh
>
> ---
> v3: rebase to other changes earlier in series
> ---
> include/bl
On 23 Apr 2016, at 00:40, Eric Blake wrote:
> Rather than open-coding NBD_REP_SERVER, reuse the code we
> already have by adding a length parameter. Additionally,
> the refactoring will make adding NBD_OPT_GO in a later patch
> easier.
>
> Signed-off-by: Eric Blake
Revie
ic Blake
> Reviewed-by: Alex Bligh
>
> ---
> v3: rebase, tweak a debug message
> ---
> include/block/nbd.h | 29 +-
> nbd/nbd-internal.h | 2 +-
> nbd/client.c| 250 ++--
> 3 files changed, 129 insertions(+)
return -EINVAL;
> + /* NBD spec says we must reply before disconnecting,
> + * but that we must also tolerate guests that don't
> + * wait for our reply. */
> +ret = nbd_negotiate_send_rep(client->ioc, NBD_REP_ACK,
> + clientflags);
> +if (!ret) {
> +ret = -EINVAL;
> +}
> +return ret;
>
> case NBD_OPT_EXPORT_NAME:
> return nbd_negotiate_handle_export_name(client, length);
> --
> 2.5.5
>
>
--
Alex Bligh
ing when a server
> requires TLS but does not have NBD_OPT_GO!), and on success
> it provides at least as much info as NBD_OPT_EXPORT_NAME sends.
>
> Signed-off-by: Eric Blake
>
Reviewed-by: Alex Bligh
> ---
> v3: revamp to match latest version of NBD protocol
> ---
>
On 23 Apr 2016, at 00:40, Eric Blake wrote:
> Upstream NBD protocol recently added the ability to efficiently
> write zeroes without having to send the zeroes over the wire,
> along with a flag to control whether the client wants a hole.
>
> Signed-off-by: Eric Blake
Reviewed
falling back to the obvious
> write of lots of zeroes if we return -ENOTSUP because the server
> does not have WRITE_ZEROES.
>
> Signed-off-by: Eric Blake
Reviewed-by: Alex Bligh
>
> ---
> v3: rebase, tell block layer about our support
> ---
> block/nbd-
Eric,
See my message on nbd-general today re the necessity (or not)
of getting NBD_OPT_BLOCK_SIZE first; it may be just that you
can assume 512 is OK.
Otherwise
Reviewed-by: Alex Bligh
Alex
On 23 Apr 2016, at 00:40, Eric Blake wrote:
> The upstream NBD Protocol has defined a new extens
LOG("Export size %" PRId64 " too large for 32-bit kernel",
> info->size);
> return -E2BIG;
> }
> @@ -724,18 +784,18 @@ int nbd_init(int fd, QIOChannelSocket *sioc,
> NbdExportInfo *info)
> return -serrno;
> }
>
> -TRACE("Setting block size to %lu", (unsigned long)BDRV_SECTOR_SIZE);
> +TRACE("Setting block size to %lu", sector_size);
>
> -if (ioctl(fd, NBD_SET_BLKSIZE, (unsigned long)BDRV_SECTOR_SIZE) < 0) {
> +if (ioctl(fd, NBD_SET_BLKSIZE, sector_size) < 0) {
> int serrno = errno;
> LOG("Failed setting NBD block size");
> return -serrno;
> }
>
> TRACE("Setting size to %lu block(s)", sectors);
> -if (size % BDRV_SECTOR_SIZE) {
> -TRACE("Ignoring trailing %d bytes of export",
> - (int) (size % BDRV_SECTOR_SIZE));
> +if (info->size % sector_size) {
> +TRACE("Ignoring trailing %" PRId64 " bytes of export",
> + info->size % sector_size);
> }
>
> if (ioctl(fd, NBD_SET_SIZE_BLOCKS, sectors) < 0) {
> --
> 2.5.5
>
>
--
Alex Bligh
on't play
nicely with all the other stuff you've been doing).
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
3.9 and later servers.
I'm going to find a minimal patch to nbd-client and offer that
to Ubuntu / Debian.
This message is here in part so I have something to point them at
on the mailing list :-)
--
Alex Bligh
;t know whether it is zero or not
And '000' is not permitted!
etc.
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
On 24 Mar 2016, at 11:58, Paolo Bonzini wrote:
>
> On 24/03/2016 11:32, Alex Bligh wrote:
>>>> Now I'm not saying we
>>>> need to fully define what it means for a part of the backend to be
>>>> "dirty" or not. It's ok
On 24 Mar 2016, at 12:32, Paolo Bonzini wrote:
> On 24/03/2016 13:17, Alex Bligh wrote:
>>>>>> * unallocated
>>>>>> * zero
>>>>>> * non-zero
>>>>>>
>>>>>> So the possible replies are a bitfield
ere is no
error.
>> I could write up a negotiation of global flags for structured reply
>> lengths as an extension proposal, if you think it is worth it.
>
> I think it is worth it...
+1 - for NBD_CMD_READ too
--
Alex Bligh
ommand flag that would allow the server to skip read chunks
> + for portions of the file that read as all zeroes.
> +
> ## About this file
>
> This file tries to document the NBD protocol as it is currently
> --
> 2.5.5
>
>
> --
> Transform Data into Opportunity.
> Accelerate data analysis in your applications with
> Intel Data Analytics Acceleration Library.
> Click to learn more.
> http://pubads.g.doubleclick.net/gampad/clk?id=278785471&iu=/4140
> ___
> Nbd-general mailing list
> nbd-gene...@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/nbd-general
>
--
Alex Bligh
hat if (when?) we add a flag for allowing the
> server to skip read chunks on holes, we'll have to tweak the wording to
> allow the server to send fewer chunks than the client's length, where
> the client must then assume zeroes for all chunks not received.
Or alternatively a chu
xhyve
has support for this, and its license is described as "BSD" (that's it). Is
that Qemu GPL compatible? Though I suspect a clean implementation may be easier.
--
Alex Bligh
Peter,
On 6 Jun 2016, at 13:43, Peter Maydell wrote:
> Not that I am aware of.
>
> Previous discussion:
> https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg03086.html
Thanks. Missed that due to "Framwork" spelling!
--
Alex Bligh
On 6 Jun 2016, at 13:29, Alex Bligh wrote:
> Is anyone working on support for hypervisor.framework (OS-X's equivalent to
> kvm as far as I can see)?
>
> If not, I might have a go in my copious spare time (cough) and if anyone
> would be interesting in helping, or giving
t broken way to do things (IIRC).
This is actually what nbd-server.c does in the threaded handler:
https://github.com/yoe/nbd/blob/master/nbd-server.c#L1468
For amusement value, the non-threaded handler (which is not used
any more) does not send any payload on an error:
https://github.com/yoe
On 14 Jun 2016, at 14:32, Paolo Bonzini wrote:
>
> On 13/06/2016 23:41, Alex Bligh wrote:
>> That's one of the reasons that there is a proposal to add
>> STRUCTURED_READ to the spec (although I still haven't had time to
>> implement that for qemu), so that we
been reasonably well known (I wrote about it
at least 3 years ago), that the current implementation
(reference + kernel) does not cope well with errors
on reads, so I'm guessing one is just trading one
set of brokenness for another. So I'm pretty relaxed
about what goes in qemu.
--
Alex Bligh
yload.
>
> In other words, the current behaviour of qemu is correct, is now
> documented to be correct, and should not be changed.
So what should those servers do (like 2 of mine) which don't buffer
the entire read, if they get an error having already sent some data?
--
Alex Bligh
, then not sending any payload, which is a
nonsense.
Perhaps this should read "If an error occurs, the server MUST either initiate
a hard disconnect before the entire payload has been sent or
set the appropriate code in the error field and send the response header
without any payload." if we want to go down this route.
--
Alex Bligh
XRSTOR (by writing the
XSAVE region with the appropriate hv_ call) and ignore all the
other register futzing? Or is it more complicated than that
because (presumably) something sets up register states in the
cpu->regs, cpu->sregs areas? I'm a bit confused as to how all
this works to be honest. Any ideas / docs to point to?
--
Alex Bligh
rs and FPState (for instance) appear to be in an opaque format:
https://developer.apple.com/library/mac/documentation/Hypervisor/Reference/Hypervisor_Reference/index.html#//apple_ref/c/func/hv_vcpu_read_fpstate
--
Alex Bligh
R_SIZE);
>> rc = -EINVAL;
>>
>
> --
> Eric Blake eblake redhat com+1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
> --
> Mobile security can be enabling,
Or does
>> fstrim work some other way?
>
> No extension in play. The kernel is obeying NBD_FLAG_SEND_TRIM, which
> is in the normative standard, and unrelated to the INFO/BLOCK_SIZE
> extensions.
My mistake. I was confusing 'WRITE_ZEROES' with 'TRIM'.
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
eed to do *some* breaking up of requests.
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
st
some breaking up.
--
Alex Bligh
man7.org/linux/man-pages/man1/fallocate.1.html
As it takes length and offset in TB, PB, EB, ZB and YB, it
seems to be 64 bit aware :-)
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
irrelevant to
your current problem, which is how this would work /with/ the block
size extensions, as Eric is in the process of implementing them).
> I do agree with your point number 3, obviously if the lenght
> field type doesn't allow something bigger than a u32, then the kernel has
> to do some breaking up in that case.
--
Alex Bligh
nal).
...
at this rate you'd be better with a list of command / maximum size
tuples!
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
On 11 May 2016, at 22:12, Wouter Verhelst wrote:
> On Tue, May 10, 2016 at 04:38:29PM +0100, Alex Bligh wrote:
>> On 10 May 2016, at 16:29, Eric Blake wrote:
>>> So the kernel is currently one of the clients that does NOT honor block
>>> sizes, and as such, servers
On 11 May 2016, at 23:39, Eric Blake wrote:
> Fix several corner-case bugs in our implementation of the NBD
> protocol, both as client and as server.
I thought I'd added a Reviewed-By: line to more of these before.
On a very very quick look, they all look good to me.
--
Alex Bligh
On 11 May 2016, at 22:06, Wouter Verhelst wrote:
> On Tue, May 10, 2016 at 04:08:50PM +0100, Alex Bligh wrote:
>> What surprises me is that a release kernel is using experimental
>> NBD extensions; there's no guarantee these won't change. Or does
>> fstrim work
failing would be 'this server
doesn't have anything it can export'.
>> Naturally the protocol document
>> (https://github.com/yoe/nbd/blob/master/doc/proto.md) isn't clear on
>> this case.
>
> You're right that we may also want to tweak the NBD protocol to make
> this interoperability point obvious.
I can't actually see the issue here. It explains what needs to be
implemented by the server, and that includes NBD_OPT_LIST. Very
happy to add some clarity, but I'm not sure where it's needed.
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
On 17 May 2016, at 16:52, Eric Blake wrote:
> On 05/17/2016 09:22 AM, Alex Bligh wrote:
>
>>>> nbdkit's implemention of NBD_OPT_LIST returns an error, because there
>>>> is no such thing as a list of export names supported (in effect nbdkit
>>>>
On 17 May 2016, at 16:54, Richard W.M. Jones wrote:
> On Tue, May 17, 2016 at 04:22:06PM +0100, Alex Bligh wrote:
>> nbdkit is non-compliant in that case. Support of NBD_OPT_LIST is
>> compulsory, even if you support it by returning a nameless export
>> (or default). Moreo
fix it, so I think
we should fix the software to confirm to the standard, not the
standard to conform to the software.
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
we need a specific number, but I think a one sentence
statement that servers are in general permitted to disconnect clients
which are behaving in a denial of service attack type manner would be
useful.
Given nbdkit only ever has one export, it seems eminently reasonable
for it to use a lower num
er from the server refusing to provide information? We don't
permit short reads etc.
> . The server SHOULD use different
> +*status* values between consecutive descriptors, and SHOULD use
> +descriptor lengths that are an integer multiple of 512 bytes where
> +possible (the first and last descriptor of an unaligned query being
> +the most obvious places for an exception).
Surely better would be an an integer multiple of the minimum block
size. Being able to offer bitmap support at finer granularity than
the absolute minimum block size helps no one, and if it were possible
to support a 256 byte block size (I think some floppy disks had that)
I see no reason not to support that as a granularity.
--
Alex Bligh
ow a hole) might need backing up).
So don't we need multiple independent lists of extents? Of course a server
might *implement* them under the hood with separate bitmaps or one big
bitmap, or no bitmap at all (for instance using file extents on POSIX).
--
Alex Bligh
16TB disk, as it might be very large! Even if the client
worked at e.g. a 64MB level (where they'd get a maximum of 1024 extents per
reply), this isn't going to noticeably increase the round trip timing. One
issue here is that to determine a 'reasonable' size, the client needs to know
the minimum length of any extent.
I think the answer is probably a 'maximum number of extents' in the request
packet.
Of course with statuses in extent, the final extent could be represented as 'I
don't know, break this bit into a separate request' status.
--
Alex Bligh
at all (well, that's my excuse
anyway).
> Anyway, I hope I am being useful and just not more confounding. It seems
> to me that we're having difficulty conveying precisely what it is we're
> trying to accomplish, so I hope that I am making a good effort in
> elaborating on our goals/requirements.
Yes absolutely. I think part of the challenge is that you are quite
reasonably coming at it from the point of view of qemu's particular
need, and I'm coming at it from 'what should the nbd protocol look
like in general' position, having done lots of work on the protocol
docs (though I'm an occasional qemu contributor). So there's necessarily
a gap of approach to be bridged.
I'm overdue on a review of Wouter's latest patch (partly because I need
to re-diff it against the version with no NBD_CMD_BLOCK_STATUS in),
but I think it's a bridge worth building.
--
Alex Bligh
I tried to pick some QEMU-like ones, but I am sure there are
examples that would work outside of QEMU.
--
Alex Bligh
s Program for Intel Xeon Phi Processors
> Access to Intel Xeon Phi processor-based developer platforms.
> With one year of Intel Parallel Studio XE.
> Training and support from Colfax.
> Order your platform today.http://sdm.link/xeonphi
> ___
> Nbd-general mailing list
> nbd-gene...@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/nbd-general
>
--
Alex Bligh
ssue passing complete
zero status back to the client if it's so obvious from a stat().
--
Alex Bligh
> On 6 Dec 2016, at 08:46, Alex Bligh wrote:
>
> I would support this.
>
> In fact the patch is sufficiently simple I think I'd merge this
> into extension-write-zeroes then merge that into master.
Hence:
Reviewed-By: Alex Bligh
--
Alex Bligh
> On 2 Dec 2016, at 18:45, Alex Bligh wrote:
>
> Thanks. That makes sense - or enough sense for me to carry on commenting!
I finally had some time to go through this extension in detail. Rather
than comment on all the individual patches, I squashed them into a single
commit, did a &#
too and a lot of uppercase will come to the code =(
I agree
--
Alex Bligh
uld read as
> all-zeroes) is not an invalid thing for a server to set. The spec here
> clarifies what a client should do with that information if it gets it
> (i.e., "don't read it, it doesn't contain anything interesting").
That's fair enough until the last bit in brackets. Rather than saying
a client SHOULD NOT read it, it should simply say that a read on
such areas will succeed but the data read is undefined (and may
not be stable).
> My WIP patch moves this out from the (older) "BLOCK_STATUS extension"
> section and into the main body of the spec. It also makes a few changes
> in wording as per what Vladimir suggested, and I was working on an
> NBD_OPT_LIST_META_CONTEXT rather than an NBD_OPT_META_CONTEXT
> negotiation option, with the idea that I'd add an OPT_ADD_META_CONTEXT
> and an OPT_DEL_META_CONTEXT later. Your idea of using a SET has merit
> though, so I'll update it to that effect.
>
> It already removed the two bits that BASE:allocation doesn't use, and
> makes a few other changes as well. I haven't had the time to finish it
> and send it out for review though, but I'll definitely include your
> comments now.
Thanks.
--
Alex Bligh
t when TRIM cannot guarantee reads-as-zero.
Yes. It was actually exactly that discussion I was trying to remember.
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
> On 14 Jan 2017, at 14:48, Wouter Verhelst wrote:
>
> On Thu, Jan 12, 2017 at 06:56:42PM +0000, Alex Bligh wrote:
>> My preferred way to do this would be essentially to allow NBD_OPT_INFO
>> to be sent (wrapped appropriately) during the main transmission phase.
>>
avily.
I'm not sure sending 4,096 items for an empty 16TB disk is any great hardship
to be honest.
--
Alex Bligh
citly say people should know the implications
of using extensions in shipping code - specifically that the
specification may change!
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
> On 3 Oct 2017, at 15:45, Paolo Bonzini wrote:
>
> Alex (both of them), Sergio, anyone else who can help?
Very interested in this (and thanks!) but it will be a while
before I have a sensible number of cycles available to play
with this one again.
--
Alex Bligh
d, where space is
> at a premium.
I did suggest a few non-Qemu uses (see below). I think it might be
an idea if the reference implementation supported it before
merging (which per below should be trivial).
--
Alex Bligh
> Begin forwarded message:
>
> From: Alex Bligh
&g
> +status of 0 for any block, although the server SHOULD return
> +additional status values when they can be easily detected.
> +
> +If an error occurs, the server SHOULD set the appropriate error
> +code in the error field of an error chunk. However, if the error
> +does not involve invalid usage (such as a request beyond the bounds
> +of the file), a server MAY reply with a single block status
> +descriptor with *length* matching the requested length, and *status*
> +of 0 rather than reporting the error.
> +
> +A client MAY initiate a hard disconnect if it detects that the
> +server has sent an invalid chunk. The server SHOULD return `EINVAL`
> +if it receives a `NBD_CMD_BLOCK_STATUS` request including one or
> +more sectors beyond the size of the device.
> +
> * Other requests
>
> Some third-party implementations may require additional protocol
>
> --
> < ron> I mean, the main *practical* problem with C++, is there's like a dozen
> people in the world who think they really understand all of its rules,
> and pretty much all of them are just lying to themselves too.
> -- #debian-devel, OFTC, 2016-02-12
> --
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, SlashDot.org!
> http://sdm.link/slashdot___
> Nbd-general mailing list
> nbd-gene...@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/nbd-general
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
l
> + namespaces by simple request to the mailinglist.
Surely also we need to specify multiple queries?
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
think the easy way to do this would be to have context IDs as a payload
to the command, like they are with NBD_CMD_WRITE, but the payload
is currently always defined to be of the length specified within the
length section.
The question really is whether we should fix this silly protocol limitation.
I don't think it's as bad as the structured reply fix, and could conceivably
go in there.
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
t it does not affect that?
How about:
"If a server supports the `base:allocation` metadata context, then writing
to an extent which has `NBD_STATE_HOLE` clear MUST NOT fail with ENOSPC
unless for reasons specified in the definition of another context."
>>> +For the `base:allocation` context, the remainder of the flags field is
>>> +reserved. Servers SHOULD set it to all-zero;
>>
>> Surely if we want to reserve them for extension, we need "Servers
>> MUST set it to all-zero"
>
> No, SHOULD, otherwise a future extension which adds meaning to those
> bits will suddenly become incompatible with this spec. Think about it
> ;-)
I did! If there is a future extension, it will change the spec to
incorporate those bits, so they won't be included within 'the
remainder' any more.
> (feel free to update the branch with those suggestions I've not NAK'd,
> as I think they make sense...)
OK. May take a look later.
--
Alex Bligh
till not be an
> error for the server to return ENOSPC)
All of this suggests 'SHOULD NOT' would be more appropriate than
'MUST NOT'.
--
Alex Bligh
ed to zero
length query) list all contexts, as absence of any query is now simple.
* Move definition of namespaces in the document to somewhere more appopriate.
* Various other minor changes as discussed on the mailing list
Signed-off-by: Alex Bligh
---
doc/proto.md
ich might (in theory) return a list of blocks which are newer than
the given timestamp. It would clearly be impossible to return all such
contexts. I wonder if we should carve out an exception here.
--
Alex Bligh
sal) is actually fine.
#1 seems too heavy
#2 also seems pretty heavyweight - adding a whole new info command for one
bit
#3 is pretty simple, but carries the disadvantage that you won't be able
to provide a reference implementation without also putting NBD_OPT_GO
support into the reference implementation. Oh hang on, perhaps that's
an advantage :-)
So I'd either go with #0 or #3.
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
> On 14 Dec 2016, at 16:58, Eric Blake wrote:
>
> s/botht he/both the/
Thanks - fixed
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
801 - 900 of 935 matches
Mail list logo
