Re: [Qemu-devel] [PATCH v3 11/13] nbd: share some nbd entities to be reused in block/nbd-client.c

[Eric Blake]

On 10/12/2017 04:53 AM, Vladimir Sementsov-Ogievskiy wrote:
> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> ---
>  include/block/nbd.h | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  nbd/nbd-internal.h  | 25 -------------------------
>  nbd/client.c        | 32 --------------------------------
>  3 files changed, 48 insertions(+), 57 deletions(-)
> 

Another thing I noticed looking at this patch:

> diff --git a/include/block/nbd.h b/include/block/nbd.h
> index dd261f66f0..09e4592971 100644
> --- a/include/block/nbd.h
> +++ b/include/block/nbd.h
> @@ -77,6 +77,9 @@ typedef struct NBDStructuredReplyChunk {
>      uint32_t length; /* length of payload */
>  } QEMU_PACKED NBDStructuredReplyChunk;
>  
> +#define NBD_SIMPLE_REPLY_MAGIC      0x67446698
> +#define NBD_STRUCTURED_REPLY_MAGIC  0x668e33ef

We have some churn here, as we defined this earlier in the series.
Also, in an ideal world (although I don't know if we're quite there), we
should be able to backport patches for JUST the server, or for JUST the
client, in isolation, to talk to an independent implementation on the
other side of the wire.  To do that, it may be better to define all our
new constants in a standalone patch, rather than embedded as part of the
server implementation in 9/13.

I think at this point, I will take 1-8 as amended, and even prepare a
pull request for those, and then post a v4 series based on your work but
with some things moved around, and get consensus on my changes, before
worrying about the pull request for the second half.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature