Re: [Qemu-devel] [PATCH 08/18] nbd: Limit nbdflags to 16 bits

[Alex Bligh]

On 8 Apr 2016, at 23:05, Eric Blake <address@hidden> wrote:

> Rather than asserting that nbdflags is within range, just give
> it the correct type to begin with :)  nbdflags corresponds to
> the per-export portion of NBD Protocol "transmission flags", which
> is 16 bits in response to NBD_OPT_EXPORT_NAME and NBD_OPT_GO.
> 
> Signed-off-by: Eric Blake <address@hidden>


Reviewed-by: Alex Bligh <address@hidden>

> ---
> include/block/nbd.h |  2 +-
> nbd/server.c        | 10 ++++------
> qemu-nbd.c          |  2 +-
> 3 files changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/include/block/nbd.h b/include/block/nbd.h
> index 2c61901..42fd670 100644
> --- a/include/block/nbd.h
> +++ b/include/block/nbd.h
> @@ -105,7 +105,7 @@ typedef struct NBDExport NBDExport;
> typedef struct NBDClient NBDClient;
> 
> 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);
> void nbd_export_close(NBDExport *exp);
> void nbd_export_get(NBDExport *exp);
> diff --git a/nbd/server.c b/nbd/server.c
> index 93c077e..c8666ab 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -63,7 +63,7 @@ struct NBDExport {
>     char *name;
>     off_t dev_offset;
>     off_t size;
> -    uint32_t nbdflags;
> +    uint16_t nbdflags;
>     QTAILQ_HEAD(, NBDClient) clients;
>     QTAILQ_ENTRY(NBDExport) next;
> 
> @@ -525,8 +525,8 @@ static coroutine_fn int nbd_negotiate(NBDClientNewData 
> *data)
>     NBDClient *client = data->client;
>     char buf[8 + 8 + 8 + 128];
>     int rc;
> -    const int myflags = (NBD_FLAG_HAS_FLAGS | NBD_FLAG_SEND_TRIM |
> -                         NBD_FLAG_SEND_FLUSH | NBD_FLAG_SEND_FUA);
> +    const uint16_t myflags = (NBD_FLAG_HAS_FLAGS | NBD_FLAG_SEND_TRIM |
> +                              NBD_FLAG_SEND_FLUSH | NBD_FLAG_SEND_FUA);
>     bool oldStyle;
> 
>     /* Old style negotiation header without options
> @@ -556,7 +556,6 @@ static coroutine_fn int nbd_negotiate(NBDClientNewData 
> *data)
> 
>     oldStyle = client->exp != NULL && !client->tlscreds;
>     if (oldStyle) {
> -        assert ((client->exp->nbdflags & ~65535) == 0);
>         stq_be_p(buf + 8, NBD_CLIENT_MAGIC);
>         stq_be_p(buf + 16, client->exp->size);
>         stw_be_p(buf + 26, client->exp->nbdflags | myflags);
> @@ -585,7 +584,6 @@ static coroutine_fn int nbd_negotiate(NBDClientNewData 
> *data)
>             goto fail;
>         }
> 
> -        assert ((client->exp->nbdflags & ~65535) == 0);
>         stq_be_p(buf + 18, client->exp->size);
>         stw_be_p(buf + 26, client->exp->nbdflags | myflags);
>         if (nbd_negotiate_write(client->ioc, buf + 18, sizeof(buf) - 18) !=
> @@ -807,7 +805,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 c2e4d3f..8880ac3 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -454,7 +454,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