reject the command with `EOVERFLOW`. The
>>> +`EOVERFLOW` error MUST NOT be used if the `NBD_CMD_FLAG_DF` flag
>>> +was not set, or if the requested length is no larger than 65,536.
>
> I'm also wondering if the wording should be switched along the lines of:
>
> If the flag is set and the server deems the request to be too large, the
> server MAY reject the command with `EOVERFLOW`; however, the server MUST
> NOT reject a request that is no larger than 65,536 bytes in this manner.
That's clearer.
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
et this
and it would be undesirable for the server to create a 'hole' using
'trim' type technology, even when the client doesn't specify it?
I suspect there are already some backends (e.g. ceph on qemu-nbd) which
will effectively do a 'trim' if you write 4k of zeroes even under
current circumstances.
IE why not always permit trimming PROVIDED the data always reads back
as zero? This would be far simpler.
--
Alex Bligh
On 31 Mar 2016, at 14:55, Paolo Bonzini wrote:
> On 31/03/2016 15:53, Alex Bligh wrote:
>>>> +If the flag `NBD_CMD_FLAG_MAY_TRIM` was set by the client in the
>>>> command
>>>> +flags field, the server MAY use trimming to zero out the area, but
Clearly set out NBDMAGIC, not the name of a constant equal to
some value. Set out the value in hex as well.
Signed-off-by: Alex Bligh
---
doc/proto.md | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/doc/proto.md b/doc/proto.md
index c1e05c5..7994076 100644
--- a/doc
NBD_CMD_FLAG_FUA is defined as 1<<0 in the documentation, but
1<<16 in nbd.h. It is not used anywhere within the code.
1<<16 cannot work as the flags word is only 16 bits long.
It is doubtful whether anyone is using NBD_CMD_FLAG_FUA
at the moment in any case.
Signed-off-by: Alex
On 31 Mar 2016, at 19:15, Alex Bligh wrote:
> It is doubtful whether anyone is using NBD_CMD_FLAG_FUA
> at the moment in any case.
Drat. I spoke too soon. Qemu uses it, but presumably from its
own .h file.
However, it's now nonsensical having it defined as 1<<16 in a
16 b
Your fix looks correct.
OK. And the wrongness hasn't yet got into /usr/include/linux/nbd.h
or include/uapi/linux/nbd.h ; these have no reference to FUA at all,
even though it does have NBD_FLAG_SEND_FLUSH, which is odd as I added
both flags at the same time.
So I'm guessing it's safe.
A on read should bypass any local read cache, though
that is not part of the spec currently.
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
"flush any
write for that block first"? The first is subtly different if the
file is remote and being accessed by multiple people (e.g. NFS, Ceph etc.)
> even if we aren't quite sure
> what to document of those flags. And that means qemu is correct, and
> the NBD protocol has a bug. Since you contributed the FUA flag, is that
> something you can try to improve?
Yeah. My mess so I should clean it up. I think FUA should be valid
on essentially everything.
I think I might wait until structured replies is in though!
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
le since I looked at nbd kernel side.
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
Improve the documentation of NBD_CMD_FLUSH and NBD_CMD_FLAG_FUA. Specifically
the latter may be set on any command, and its semantics on commands other
than NBD_CMD_WRITE need explaining. Further, explain how these relate to
reordering of commands.
Signed-off-by: Alex Bligh
---
doc/proto.md
NUL terminated).
Signed-off-by: Alex Bligh
---
doc/proto.md | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/doc/proto.md b/doc/proto.md
index c1e05c5..7729051 100644
--- a/doc/proto.md
+++ b/doc/proto.md
@@ -292,7 +292,8 @@ of the newstyle negotiation.
haggling, and
On 1 Apr 2016, at 08:43, Paolo Bonzini wrote:
>
> On 31/03/2016 22:17, Alex Bligh wrote:
>>>> In qemu, read+FUA just triggers blk_co_flush() prior to reading; but
>>>> that's the same function it calls for write+FUA.
>> That's harmless, but un
bit is not natively supported the block
> layer turns it into an empty REQ_FLUSH request after the actual write.
That's pretty ambiguous as to whether you need to handle it for reads
as well. If it does read+FUA means that the read needs an fdatasync()
type operation to happen.
--
Alex Bligh
r they have been PROCESSED without a reply.
Personally I think the SHOULD clause is pretty pointless and is
unnecessary, but that's where the conversation got to n years
ago I believe.
Happy to delete the last sentence if that's wrong.
--
Alex Bligh
On 1 Apr 2016, at 08:59, Wouter Verhelst wrote:
> On Thu, Mar 31, 2016 at 07:15:32PM +0100, Alex Bligh wrote:
>> NBD_CMD_FLAG_FUA is defined as 1<<0 in the documentation, but
>> 1<<16 in nbd.h. It is not used anywhere within the code.
>
> Yes it is:
>
>
l
semantic was meant to be the kernel bio layer semantic, and (as per message to
Paolo) the kernel documentation is less than helpful in whether it can be set
on a read bio.
--
Alex Bligh
Improve the documentation of NBD_CMD_FLUSH and NBD_CMD_FLAG_FUA. Specifically
the latter may be set on any command, and its semantics on commands other
than NBD_CMD_WRITE need explaining. Further, explain how these relate to
reordering of commands.
Signed-off-by: Alex Bligh
---
doc/proto.md
Clients MUST NOT set unknown client flags. Currently this is
permitted (but 'SHOULD NOT' be done), with the result that the
server MUST drop the connection if it happens. This in effect
gives the client an inappropriate way to close the connection.
Signed-off-by: Alex Bligh
---
doc/pr
On 1 Apr 2016, at 10:43, Wouter Verhelst wrote:
> On Fri, Apr 01, 2016 at 10:32:50AM +0100, Alex Bligh wrote:
>> #define NBD_CMD_MASK_COMMAND 0x
>> -#define NBD_CMD_FLAG_FUA (1<<16)
>> +#define NBD_CMD_SHIFT (16)
>> +#define NBD_CMD_FLAG_FUA ((1 <&l
On 1 Apr 2016, at 11:10, Wouter Verhelst wrote:
> On Fri, Apr 01, 2016 at 10:28:03AM +0100, Alex Bligh wrote:
>>
>> On 1 Apr 2016, at 09:35, Wouter Verhelst wrote:
>>
>>>> +* All write commands (that includes both `NBD_CMD_WRITE` and
>>>> + `
Restore formatting and correct name of 'length'
Signed-off-by: Alex Bligh
---
doc/proto.md | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/doc/proto.md b/doc/proto.md
index 8c83382..03dfe2b 100644
--- a/doc/proto.md
+++ b/doc/proto.md
@@ -501,8 +501,8 @@ The
NBD_CMD_FLAG_FUA is defined as 1<<0 in the documentation, but
1<<16 in nbd.h.
The code currently treats the command as a 32 bit quantity
and masks this off. This is confusing. Until such time as the
code is fixed up, make it obvious this isn't really bit 16.
Signed-off-by: Alex
Clearly set out NBDMAGIC, not the name of a constant equal to
some value. Set out the value in hex as well.
Document the newstyle magic number is "IHAVEOPT".
Signed-off-by: Alex Bligh
---
doc/proto.md | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/do
On 1 Apr 2016, at 12:11, Wouter Verhelst wrote:
> I don't think it is at all useful to tell the server what it should do
> in that situation.
Agree. I will delete it.
--
Alex Bligh
the first one. On the other hand if we
take route (a) above, we can relatively easily add a
'meaning' later as people can't have been relying on it
working.
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
e and the kernel doesn't use FUA
at all; I realise that is not an exhaustive list.
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
On 1 Apr 2016, at 16:12, Alex Bligh wrote:
> as qemu doesn't use FUA on write and the kernel doesn't use FUA
"as qemu doesn't use FUA other than on write" - sorry
> at all; I realise that is not an exhaustive list.
--
Alex Bligh
signature.asc
Descripti
ent talks to qemu server); and at the same time
> out to fix NBD server to silently ignore FUA on flush (fix the breakage
> between the pairing).
+1.
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
On 1 Apr 2016, at 20:39, Eric Blake wrote:
> A couple of typos, odd formatting, and missing words made it into
> the structured read spec, and several potential ambiguous situations
> were worth rewording for clarity.
>
> Signed-off-by: Eric Blake
> Signed-off-by: Alex B
er execution.
>
> The name NO_HOLE is also slightly nicer at expressing the fact
> that there is no dependency on whether NBD_CMD_TRIM is supported.
>
> Also tweak a couple of formatting issues for consistency (for
> example, only reserve a bit number in one place).
>
>
Wouter,
On 2 Apr 2016, at 13:57, Wouter Verhelst wrote:
> On Fri, Apr 01, 2016 at 10:54:42AM +0100, Alex Bligh wrote:
>> Improve the documentation of NBD_CMD_FLUSH and NBD_CMD_FLAG_FUA. Specifically
>> the latter may be set on any command, and its semantics on command
e in any case, as the underlying filing system
could create a hole.
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
ing a hole and risking fragmentation or
> future ENOSPC even when the client explicitly wanted to write
> zeroes rather than a hole. So it makes sense to let the new
> NBD_CMD_FLAG_NO_HOLE work for WRITE as well as WRITE_ZEROES.
>
> Signed-off-by: Eric Blake
Reviewe
return a monotonic 64 bit counter of the number of
writes to the disk since some arbitrary time, and provide a 'GETDIRTY'
command that returned all blocks with a monotonic counter greater than that.
That way I could precisely get the writes that were executed since
any particular read. You'd allow it to be 'slack' and include things
in that list that might not have changed (i.e. false positives) but
not false negatives.
--
Alex Bligh
rotocol (see my strawman) but
I'd like to know it's at least possibly useful.
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
ed to qemu myself), but perhaps it might be better then,
rather than call it 'dirtiness' to make it some named attribute
for private client/server communication.
This again makes me think this should be a different
command from something which is obviously useful and
comprehensible to more than one server/client (i.e.
allocation).
--
Alex Bligh
were two different commands, they could each run at their
natural block size.
--
Alex Bligh
ugh I'd suggest the
bitmap had an ID other than a number 0..15 so other people
can use it too.
--
Alex Bligh
(1) or (2), but not both. I see. I misread
that. I still think Denis's idea of selecting a bitmap to query works better.
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
tured writes - right now, you can write a
... I agree this is almost inventing 'structured writes' :-)
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
On 4 Apr 2016, at 22:04, Denis V. Lunev wrote:
> and opinion of Alex was that all 3 bits could be set in reply to
> NBD_CMD_BLOCK_STATUS
> with NBD_CMD_FLAG_STATUS_DIRTY set.
>
> This confused him. This confuses me too.
Yeah that was precisely how I got confused.
--
Alex Bligh
On 4 Apr 2016, at 22:25, Eric Blake wrote:
> On 04/04/2016 03:07 PM, Alex Bligh wrote:
>>
>> On 4 Apr 2016, at 21:26, Eric Blake wrote:
>>
>>> Huh? The current proposal _requires_ these to be separate queries. You
>>> cannot query dirtiness at the sam
s allocated on the second-level
> snapshot
This is what I originally thought Eric was proposing. The issue
is that in common servers the 'dirtiness' is at a different
allocation / blocksize from the allocation status. So you
really want 2 different bitmaps.
--
Alex Bligh
Signed-off-by: Alex Bligh
---
doc/proto.md | 51 +--
1 file changed, 41 insertions(+), 10 deletions(-)
Changes since version 2:
* Rebase on master
* Remove bogus 'SHOULD' for FLUSH in relation to writes that are in flight
but not yet
Improve the documentation of NBD_CMD_FLUSH and NBD_CMD_FLAG_FUA. Specifically
the latter may be set on any command, and its semantics on commands other
than NBD_CMD_WRITE need explaining. Further, explain how these relate to
reordering of commands.
Signed-off-by: Alex Bligh
---
doc/proto.md
On 5 Apr 2016, at 15:15, Paolo Bonzini wrote:
>
> On 04/04/2016 22:03, Alex Bligh wrote:
>> So qemu-nbdserver has some idea of 'dirtiness', and you want
>> to publish that, and the consumer is qemu (and only qemu),
>> because only qemu knows what 'dirt
gt;> reached permanent storage.
>>
>> -If the `NBD_FLAG_SEND_FUA` flag ("Force Unit Access") was set in the
>> -transmission flags field, the client MAY set the flag
>> `NBD_CMD_FLAG_FUA` in
>> -the command flags field. If this flag was set, the server MUST NOT send
>> -the reply until it has ensured that the newly-written data has reached
>> -permanent storage.
>> -
>
> You should drop this paragraph from both NBD_CMD_WRITE and
> NBD_CMD_WRITE_ZEROES, now that the flag is globally described (here, you
> only dropped it from one of the two).
Thanks, missed that.
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
FUA on not write requests
> Date: 1 April 2016 17:06:09 BST
> To: Alex Bligh
> Cc: "linux-kernel\@vger.kernel.org" ,
> nbd-gene...@lists.sourceforge.net, Eric Blake
>
> Alex Bligh writes:
>
>> I am trying to clean up the documentation of the NBD protocol.
perhaps 'non-NBD').
Of course this is made harder as the NBD_CMD_ size is not
extensible (today).
--
Alex Bligh
Improve the documentation of NBD_CMD_FLUSH and NBD_CMD_FLAG_FUA. Specifically
the latter may be set on any command, and its semantics on commands other
than NBD_CMD_WRITE need explaining. Further, explain how these relate to
reordering of commands.
Signed-off-by: Alex Bligh
---
doc/proto.md
On 5 Apr 2016, at 16:22, Paolo Bonzini wrote:
> Right. However, bugs get fixed...
Sure. I'd quite like to get this change in now though, and
we can add a treatment for 'read' later. It's the ordering stuff
which is really important, plus 'don't break current QEMU' :-)
--
Alex Bligh
in the option header
anyway.
Signed-off-by: Alex Bligh
---
doc/proto.md | 123 ---
1 file changed, 83 insertions(+), 40 deletions(-)
diff --git a/doc/proto.md b/doc/proto.md
index 35a3266..06601fb 100644
--- a/doc/proto.md
+++ b/doc/proto
Improve the documentation of NBD_CMD_FLUSH and NBD_CMD_FLAG_FUA. Specifically
the latter may be set on any command, and its semantics on commands other
than NBD_CMD_WRITE need explaining. Further, explain how these relate to
reordering of commands.
Reviewed-by: Eric Blake
Signed-off-by: Alex
reference implementation here:
https://github.com/yoe/nbd/blob/master/nbd-server.c#L1232
and as is evident it always sends a 'datasize' (aka length) 32 bit
word. Unsupported elements are replied to here:
https://github.com/yoe/nbd/blob/master/nbd-server.c#L1371
Signed-off-by: Alex Blig
Eric,
In brief I agree with all of that. I'm tempted to redo it
so it's much simpler now we all seem to agree that carrying
state is a bad thing.
Alex
On 5 Apr 2016, at 18:48, Eric Blake wrote:
> On 04/05/2016 10:38 AM, Alex Bligh wrote:
>> Amend the NBD_OPT_SE
Give slightly more information when certification loading fails.
Rather than have no information, you now get gnutls's only slightly
less unhelpful error messages.
Signed-off-by: Alex Bligh
---
crypto/tlscredsx509.c | 17 +++--
1 file changed, 11 insertions(+), 6 deletions(-)
4) gnutls installed.
$ dpkg --list | fgrep libgnutls26
ii libgnutls26:amd64 2.12.23-12ubuntu2.4 amd64 GNU TLS library - runtime
library
All the certificates are at:
https://gist.github.com/abligh/96425e20fb423d847b8fd4ead298efed
(no there's nothing secret there)
Any ideas?
--
Alex Bligh
ements are at the end.
* Make the documentation much more concise.
Signed-off-by: Alex Bligh
---
doc/proto.md | 142 ---
1 file changed, 76 insertions(+), 66 deletions(-)
Changes since v1:
* Change NBD_OPT_SELECT to be called NBD_OPT
this proposal, and perhaps let's have it as
tuples so a protocol analyzer can
parse it. If you want to put in some indicator these might
be present then fine, but I'm guesing transmission flags
could take that as a bit. But if we introduce such tuples,
there's no need to limit them to 124 bytes.
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
l 1.0.1f-1ubuntu2.16
python-crypto 2.6.1-4build1
python-openssl 0.13-2ubuntu6
python-passlib 1.5.3-0ubuntu3
python3-crypto 2.6.1-4build1
ssl-cert 1.0.33
--
Alex Bligh
27;old qemu' libraries (and besides
'old qemu' had no TLS code). I've put the output of ldd -v below
as well just in case.
--
Alex Bligh
make ./tests/test-crypto-tlssession ./tests/test-crypto-tlscredsx509
CHK version_gen.h
CCtests/test-crypto-tlssession.o
C
t; extra data has a definition other than a UTF-8 message.
I'm not sure reordering those fields was a great idea :-/
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
Currently doc/proto.md is silent on use of NBD_OPT_STARTTLS when
TLS has already been negotiated. Make it clear that this is not
permitted.
Signed-off-by: Alex Bligh
---
doc/proto.md | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/doc/proto.md b/doc/proto.md
index
lder than that. I just copied you on a patch to fix this.
Thanks. I can confirm that fixes it at least up until the point
it loads certificates. I will add Reviewed-By: and Tested-By: tags.
Can I suggest this might be suitable for qemu-stable as it's clearly
a bug fix?
--
Alex Bligh
nything else.
>
> This is important because some versions of gnutls/gcrypt
> require explicit initialization before use.
>
> Signed-off-by: Daniel P. Berrange
Reviewed-by: Alex Bligh
Tested-by: Alex Bligh
> ---
> qemu-img.c | 6 ++
> qemu-io.c | 6 ++
> qemu-
On 6 Apr 2016, at 16:35, Eric Blake wrote:
> Fix several document inconsistencies (missing references,
> rewrap long lines, address typos, improve grammar) introduced
> in recent patches.
>
> Signed-off-by: Eric Blake
Reviewed-by: Alex Bligh
Most of that mess was mine - than
On 6 Apr 2016, at 17:14, Eric Blake wrote:
> I just re-reviewed this patch. While what you have is a strict
> improvement, it is still buggy for a server that sends a UTF-8 message
> explaining the error:
Indeed. That should be fixed too. I missed that one.
--
Alex Bligh
sign
h an extension is already
> envisioned, where the client and server agree to have the server
> advertise minimum, preferred, and maximum block sizes).
>
> Signed-off-by: Eric Blake
Reviewed-by: Alex Bligh
Thanks for this. I was going to clear this up myself today
having introd
requirement on servers, and what is a requirement on
clients, separately, specifying their behaviour in a single place
in the document.
* Document the four possible modes of operation of a server.
Signed-off-by: Alex Bligh
---
doc/proto.md | 267
LECTIVETLS - explicitly permitted by the
INFO extension currently, and interoperability is fine. And this
is perfectly compatible behaviour with what I suggest.
Incidentally, the qemu client does not appear to assume the
server is 'FORCEDTLS', and IIRC from time spend staring into
Wireshark yesterday sends its NBD_OPT_LIST prior to the TLS
upgrade. I can check that if you like.
--
Alex Bligh
On 7 Apr 2016, at 13:13, Alex Bligh wrote:
> I guess it's worth documenting
> this, though I thought it was obvious.
The next version will have this section:
### Downgrade attacks
A danger inherent in any scheme relying on the negotiation
of whether TLS should be employed i
;, and IIRC from time spend staring into
>> Wireshark yesterday sends its NBD_OPT_LIST prior to the TLS
>> upgrade. I can check that if you like.
>
> If the qemu NBD client has TLS credentials set then it will
> refuse to connect to a server without TLS. The OPT_TLS is
> definitely the first thing it sends, becasue the QEMU NBD
> server will reject all options until OPT_TLS has been sent.
Yep you are right - just checked wireshark.
--
Alex Bligh
>> +If a client supports TLS, it SHOULD also support the INFO
>> +extension, and SHOULD use `NBD_OPT_GO` if available in place
>> +of `NBD_OPT_EXPORT_NAME`.
>
> add: it MUST also support the NBD_FLAG_C_FIXED_NEWSTYLE flag
See above. I'll put it up top.
>> +See the section on TLS above for futher details.
>
> s/futher/further/
thx.
>>
>> - `NBD_OPT_INFO` (6)
>>
>> @@ -489,20 +701,9 @@ case that data is an error message string suitable for
>> display to the user.
>> * `NBD_REP_ERR_TLS_REQD` (2^31 + 5)
>>
>> The server is unwilling to continue negotiation unless TLS is
>
>> +initiated first. In the case of `NBD_OPT_INFO` and `NBD_OPT_GO`
>> +this unwillingness MAY be limited to the export in question.
>
> Maybe add: "but only for a server in SELECTIVETLS mode"
Well the following line ...
>> +See the section on TLS above for further details.
... was meant to imply that! I'll make it clearer.
>>
>> In the case of `NBD_REP_SERVER`, the message's data takes on a different
>> interpretation than the default (so as to provide additional
>>
>
> and don't forget to tweak NBD_OPT_LIST to cater to skip listings of
> TLS-required exports during SELECTIVETLS mode.
OK. v2 coming up.
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
requirement on servers, and what is a requirement on
clients, separately, specifying their behaviour in a single place
in the document.
* Document the four possible modes of operation of a server.
Signed-off-by: Alex Bligh
---
doc/proto.md | 336
Eric,
(this crossed with v2)
On 7 Apr 2016, at 16:35, Eric Blake wrote:
> On 04/07/2016 06:36 AM, Alex Bligh wrote:
>>
>> On 7 Apr 2016, at 13:13, Alex Bligh wrote:
>>
>>> I guess it's worth documenting
>>> this, though I thought it was obv
advertised block sizes).
+1
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
requirement on servers, and what is a requirement on
clients, separately, specifying their behaviour in a single place
in the document.
* Document the four possible modes of operation of a server.
Signed-off-by: Alex Bligh
---
doc/proto.md | 341
requirement on servers, and what is a requirement on
clients, separately, specifying their behaviour in a single place
in the document.
* Document the four possible modes of operation of a server.
Reviewed-by: Eric Blake
Signed-off-by: Alex Bligh
---
doc/proto.md | 340
On 7 Apr 2016, at 21:35, Eric Blake wrote:
> grammar tweak:
> s/ and either /, /
sigh. fixed. Remembered to add your Reviewed-by: tag too.
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
ing the intent
> of the NBD Protocol. However, it's better to allow the client to
> continue 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
Looks right to me - untested.
>
n negotiated is documented elsewhere
and of course applies to all options other then NBD_OPT_STARTTLS.
> I'm down to just 2 findings and a side comment, which means we're close
> enough that you can add:
> Reviewed-by: Eric Blake
Thanks - will send another version anyway.
Alex
> --
> Eric Blake eblake redhat com+1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
error_setg(errp, "incorrect option name length");
> return -1;
> }
> if (namelen > 255) {
Shouldn't that be 4096? You are after all reading up to NBD_MAX_BUFFER_SIZE
(32K) of data just earlier.
Not technically the bug you are trying t
OT send this option if
>> -it has already negotiated TLS; if the server receives
>> -`NBD_OPT_STARTTLS` when TLS has already been negotiated, the server
>> -MUST send back `NBD_REP_ERR_INVALID`.
>> -
>> -This functionality has not yet been implemented by the reference
>> -implementation, but was implemented by qemu so has been moved out of
>> -the "experimental" section.
>> +The client wishes to initiate TLS.
>> +
>> +The server MUST either reply with `NBD_REP_ACK` after which
>> +point the connection is upgraded to TLS, or reply with
>> +`NBD_REP_ERR_UNSUP`.
>
> (or POLICY)
OK
> --
> < 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
>
--
Alex Bligh
On 9 Apr 2016, at 10:50, Wouter Verhelst wrote:
> So if I want to swap to qemu-nbd, I cannot also have encrypted
> connections to the same server. Got it.
AFAIK qemu-nbd only supports a single export so this isn't
really an issue.
--
Alex Bligh
ns, but where ANY
>> option works. A server with TLS but not FIXED_NEWSTYLE is pointless
>
> Actually, such a server is technically impossible ;-)
Yeah, you really want to be reading v5 of this patch set by which
time we cleared all this up :-)
--
Alex Bligh
NOT allow TLS versions older than 1.2" here.
> There are some serious flaws in older TLS versions; currently these are
> still supported by most web browsers for backwards compatibility
> reasons, but that does not apply for us.
I'd be all for that. Or certainly "SHOULD NOT support LS versions older
than 1.2 by default" (gonbdserver allows you to configure minimum and
maximum TLS versions).
--
Alex Bligh
ing the intent
> of the NBD Protocol. However, it's better to allow the client to
> continue 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
> ---
> nbd/server.c | 10
ng, and one that replies with super-huge
> lengths could cause us to temporarily allocate up to 4G memory.
> Sanity check things before blindly reading incorrectly.
>
> Signed-off-by: Eric Blake
Reviewed-by: Alex Bligh
> ---
> nbd/client.c | 23 +--
> 1 fi
On 8 Apr 2016, at 23:05, Eric Blake wrote:
> Clean up some debug message oddities missed earlier; this includes
> both typos, and recognizing that %d is not necessarily compatible
> with uint32_t.
>
> Signed-off-by: Eric Blake
Reviewed-by: Alex Bligh
> ---
&
On 8 Apr 2016, at 23:05, Eric Blake wrote:
> Add some debugging to flag servers that are not compliant to
> the NBD protocol.
>
> Signed-off-by: Eric Blake
Reviewed-by: Alex Bligh
> ---
> nbd/client.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
>
possibly behave differently
> than the client was expecting with the attempted flag.
>
> Signed-off-by: Eric Blake
Reviewed-by: Alex Bligh
> ---
> nbd/server.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/nbd/server.c b/nbd/server.c
> index 81af
ed export length as 4096, why not change
this to 4096?
Otherwise:
Reviewed-by: Alex Bligh
>
> ssize_t nbd_wr_syncv(QIOChannel *ioc,
> struct iovec *iov,
> diff --git a/nbd/client.c b/nbd/client.c
> index c834587..00f9244 100644
> --- a/nbd/client.c
> +++
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
> ---
> include/block/nbd.h | 18 --
> nbd/nbd-internal.h | 4
BD_OPT_EXPORT_NAME and NBD_OPT_GO.
>
> Signed-off-by: Eric Blake
Reviewed-by: Alex Bligh
> ---
> include/block/nbd.h | 2 +-
> nbd/server.c| 10 --
> qemu-nbd.c | 2 +-
> 3 files changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/include/
> Signed-off-by: Eric Blake
Reviewed-by: Alex Bligh
> ---
> nbd/server.c | 59 +--
> 1 file changed, 29 insertions(+), 30 deletions(-)
>
> diff --git a/nbd/server.c b/nbd/server.c
> index c8666ab..69724c9 100644
>
iate_read() which handles
> coroutine magic, but we can copy the idea into the client where
> we have places where we want to ignore data (such as the
> description tacked on the end of NBD_REP_SERVER).
>
> Signed-off-by: Eric Blake
Reviewed-by: Alex B
ric Blake
Reviewed-by: Alex Bligh
> ---
> include/block/nbd.h | 29 +-
> nbd/nbd-internal.h | 2 +-
> nbd/client.c| 250 ++--
> 3 files changed, 129 insertions(+), 152 deletions(-)
>
> diff --git a/include/block
rst-case
> string, so we may still want to take the leeway offered by SHOULD
> to force a reasonable smaller limit).
Is there a limit in qemu-world to safe stack allocation? I thought
that was (in general) only a kernel consideration? (probably my
ignorance here).
Otherwise:
possible in
> newstyle, whether or not it is fixed newstyle). It doesn't
> shave much off the wire, but we might as well implement it.
>
> Signed-off-by: Eric Blake
Reviewed-by: Alex Bligh
thanks - that was annoying me.
> ---
> include/block/nbd.h | 6 --
> n
701 - 800 of 935 matches
Mail list logo
