[Qemu-devel] [PATCHv2 1/2] proto.md: Clearly set out NBDMAGIC is the actual value

[Qemu-devel] [PATCHv2 1/2] proto.md: Clearly set out NBDMAGIC is the actual value

[Alex Bligh]

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 <alex@alex.org.uk>
---
 doc/proto.md | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/doc/proto.md b/doc/proto.md
index 03dfe2b..8376021 100644
--- a/doc/proto.md
+++ b/doc/proto.md
@@ -67,7 +67,8 @@ newstyle negotiation.
 
 #### Oldstyle negotiation
 
-S: 64 bits, `NBDMAGIC` (also known as the `INIT_PASSWD`)  
+S: 64 bits, `0x4e42444d41474943` (ASCII '`NBDMAGIC`') (also known as
+   the `INIT_PASSWD`)  
 S: 64 bits, `0x00420281861253` (`cliserv_magic`, a magic number)  
 S: 64 bits, size of the export in bytes (unsigned)  
 S: 32 bits, flags  
@@ -96,8 +97,10 @@ production purposes.
 
 The initial few exchanges in newstyle negotiation look as follows:
 
-S: 64 bits, `NBDMAGIC` (as in the old style handshake)  
-S: 64 bits, `0x49484156454F5054` (note different magic number)  
+S: 64 bits, `0x4e42444d41474943` (ASCII '`NBDMAGIC`') (as in the old
+   style handshake)  
+S: 64 bits, `0x49484156454F5054` (ASCII '`IHAVEOPT`') (note different
+   magic number)  
 S: 16 bits, handshake flags  
 C: 32 bits, flags  
 
@@ -113,7 +116,8 @@ At this point, we move on to option haggling, during which point the
 client can send one or (in fixed newstyle) more options to the server.
 The generic format of setting an option is as follows:
 
-C: 64 bits, `0x49484156454F5054` (note same newstyle handshake's magic number)  
+C: 64 bits, `0x49484156454F5054` (ASCII '`IHAVEOPT`') (note same
+   newstyle handshake's magic number)  
 C: 32 bits, option  
 C: 32 bits, length of option data (unsigned)  
 C: any data needed for the chosen option, of length as specified above.  
-- 
1.9.1

[Qemu-devel] [PATCHv2 2/2] Correct definition of NBD_CMD_FLAG_FUA

[Alex Bligh]

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 Bligh <alex@alex.org.uk>
---
 nbd.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/nbd.h b/nbd.h
index f2a32dd..732c605 100644
--- a/nbd.h
+++ b/nbd.h
@@ -38,7 +38,8 @@ enum {
 };
 
 #define NBD_CMD_MASK_COMMAND 0x0000ffff
-#define NBD_CMD_FLAG_FUA (1<<16)
+#define NBD_CMD_SHIFT (16)
+#define NBD_CMD_FLAG_FUA ((1 << 0) << NBD_CMD_SHIFT)
 
 /* values for flags field */
 #define NBD_FLAG_HAS_FLAGS	(1 << 0)	/* Flags are there */
-- 
1.9.1

Re: [Qemu-devel] [PATCHv2 1/2] proto.md: Clearly set out NBDMAGIC is the actual value

[Eric Blake]

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

On 04/01/2016 04:46 AM, Alex Bligh wrote:
> 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 <alex@alex.org.uk>
> ---
>  doc/proto.md | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
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 --]

Re: [Qemu-devel] [PATCHv2 2/2] Correct definition of NBD_CMD_FLAG_FUA

[Eric Blake]

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

On 04/01/2016 04:46 AM, Alex Bligh wrote:
> 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 Bligh <alex@alex.org.uk>
> ---
>  nbd.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
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 --]