Re: [PATCH v5 09/15] block/nvme: Replace qemu_try_blockalign0 by qemu_try_blockalign/memset

[Stefano Garzarella]

On Thu, Aug 20, 2020 at 06:58:55PM +0200, Philippe Mathieu-Daudé wrote:
> In the next commit we'll get rid of qemu_try_blockalign().
> To ease review, first replace qemu_try_blockalign0() by explicit
> calls to qemu_try_blockalign() and memset().
> 
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  block/nvme.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

> 
> diff --git a/block/nvme.c b/block/nvme.c
> index 2bd1935f951..ac6bb52043d 100644
> --- a/block/nvme.c
> +++ b/block/nvme.c
> @@ -174,12 +174,12 @@ static void nvme_init_queue(BlockDriverState *bs, 
> NVMeQueue *q,
>  
>      bytes = ROUND_UP(nentries * entry_bytes, s->page_size);
>      q->head = q->tail = 0;
> -    q->queue = qemu_try_blockalign0(bs, bytes);
> -
> +    q->queue = qemu_try_blockalign(bs, bytes);
>      if (!q->queue) {
>          error_setg(errp, "Cannot allocate queue");
>          return;
>      }
> +    memset(q->queue, 0, bytes);
>      r = qemu_vfio_dma_map(s->vfio, q->queue, bytes, false, &q->iova);
>      if (r) {
>          error_setg(errp, "Cannot map queue");
> @@ -223,11 +223,12 @@ static NVMeQueuePair 
> *nvme_create_queue_pair(BlockDriverState *bs,
>      if (!q) {
>          return NULL;
>      }
> -    q->prp_list_pages = qemu_try_blockalign0(bs,
> +    q->prp_list_pages = qemu_try_blockalign(bs,
>                                            s->page_size * NVME_QUEUE_SIZE);
>      if (!q->prp_list_pages) {
>          goto fail;
>      }
> +    memset(q->prp_list_pages, 0, s->page_size * NVME_QUEUE_SIZE);
>      qemu_mutex_init(&q->lock);
>      q->s = s;
>      q->index = idx;
> @@ -521,7 +522,7 @@ static void nvme_identify(BlockDriverState *bs, int 
> namespace, Error **errp)
>          .cdw10 = cpu_to_le32(0x1),
>      };
>  
> -    id = qemu_try_blockalign0(bs, sizeof(*id));
> +    id = qemu_try_blockalign(bs, sizeof(*id));
>      if (!id) {
>          error_setg(errp, "Cannot allocate buffer for identify response");
>          goto out;
> @@ -531,8 +532,9 @@ static void nvme_identify(BlockDriverState *bs, int 
> namespace, Error **errp)
>          error_setg(errp, "Cannot map buffer for DMA");
>          goto out;
>      }
> -    cmd.prp1 = cpu_to_le64(iova);
>  
> +    memset(id, 0, sizeof(*id));
> +    cmd.prp1 = cpu_to_le64(iova);
>      if (nvme_cmd_sync(bs, s->queues[INDEX_ADMIN], &cmd)) {
>          error_setg(errp, "Failed to identify controller");
>          goto out;
> @@ -1283,11 +1285,11 @@ static int coroutine_fn 
> nvme_co_pdiscard(BlockDriverState *bs,
>  
>      assert(s->nr_queues > 1);
>  
> -    buf = qemu_try_blockalign0(bs, s->page_size);
> +    buf = qemu_try_blockalign(bs, s->page_size);
>      if (!buf) {
>          return -ENOMEM;
>      }
> -
> +    memset(buf, 0, s->page_size);
>      buf->nlb = cpu_to_le32(bytes >> s->blkshift);
>      buf->slba = cpu_to_le64(offset >> s->blkshift);
>      buf->cattr = 0;
> -- 
> 2.26.2
> 
>