Re: [PATCH v2 04/16] hw/block/nvme: remove redundant has_sg member

[Maxim Levitsky]

On Thu, 2020-07-30 at 00:06 +0200, Klaus Jensen wrote:
> From: Klaus Jensen 
> 
> Remove the has_sg member from NvmeRequest since it's redundant.
> 
> Signed-off-by: Klaus Jensen 


Reviewed-by: Maxim Levitsky 

Best regards,
Maxim Levitsky

> ---
>  hw/block/nvme.c | 7 ++-
>  hw/block/nvme.h | 1 -
>  2 files changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index d60b19e1840f..a9d9a2912655 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -550,7 +550,8 @@ static void nvme_rw_cb(void *opaque, int ret)
>  block_acct_failed(blk_get_stats(n->conf.blk), >acct);
>  req->status = NVME_INTERNAL_DEV_ERROR;
>  }
> -if (req->has_sg) {
> +
> +if (req->qsg.nalloc) {
>  qemu_sglist_destroy(>qsg);
>  }
>  nvme_enqueue_req_completion(cq, req);
> @@ -559,7 +560,6 @@ static void nvme_rw_cb(void *opaque, int ret)
>  static uint16_t nvme_flush(NvmeCtrl *n, NvmeNamespace *ns, NvmeCmd *cmd,
>  NvmeRequest *req)
>  {
> -req->has_sg = false;
>  block_acct_start(blk_get_stats(n->conf.blk), >acct, 0,
>   BLOCK_ACCT_FLUSH);
>  req->aiocb = blk_aio_flush(n->conf.blk, nvme_rw_cb, req);
> @@ -585,7 +585,6 @@ static uint16_t nvme_write_zeros(NvmeCtrl *n, 
> NvmeNamespace *ns, NvmeCmd *cmd,
>  return NVME_LBA_RANGE | NVME_DNR;
>  }
>  
> -req->has_sg = false;
>  block_acct_start(blk_get_stats(n->conf.blk), >acct, 0,
>   BLOCK_ACCT_WRITE);
>  req->aiocb = blk_aio_pwrite_zeroes(n->conf.blk, offset, count,
> @@ -623,7 +622,6 @@ static uint16_t nvme_rw(NvmeCtrl *n, NvmeNamespace *ns, 
> NvmeCmd *cmd,
>  }
>  
>  if (req->qsg.nsg > 0) {
> -req->has_sg = true;
>  block_acct_start(blk_get_stats(n->conf.blk), >acct, 
> req->qsg.size,
>   acct);
>  req->aiocb = is_write ?
> @@ -632,7 +630,6 @@ static uint16_t nvme_rw(NvmeCtrl *n, NvmeNamespace *ns, 
> NvmeCmd *cmd,
>  dma_blk_read(n->conf.blk, >qsg, data_offset, 
> BDRV_SECTOR_SIZE,
>   nvme_rw_cb, req);
>  } else {
> -req->has_sg = false;
>  block_acct_start(blk_get_stats(n->conf.blk), >acct, 
> req->iov.size,
>   acct);
>  req->aiocb = is_write ?
> diff --git a/hw/block/nvme.h b/hw/block/nvme.h
> index 0b6a8ae66559..5519b5cc7686 100644
> --- a/hw/block/nvme.h
> +++ b/hw/block/nvme.h
> @@ -22,7 +22,6 @@ typedef struct NvmeRequest {
>  struct NvmeSQueue   *sq;
>  BlockAIOCB  *aiocb;
>  uint16_tstatus;
> -boolhas_sg;
>  NvmeCqe cqe;
>  BlockAcctCookie acct;
>  QEMUSGList  qsg;

Re: [PATCH v2 04/16] hw/block/nvme: remove redundant has_sg member

[Minwoo Im]

On Thu, Jul 30, 2020 at 7:06 AM Klaus Jensen  wrote:
>
> From: Klaus Jensen 
>
> Remove the has_sg member from NvmeRequest since it's redundant.
>
> Signed-off-by: Klaus Jensen 

Looks better than the previous one to me.

Reviewed-by: Minwoo Im 

[PATCH v2 04/16] hw/block/nvme: remove redundant has_sg member

[Klaus Jensen]

From: Klaus Jensen 

Remove the has_sg member from NvmeRequest since it's redundant.

Signed-off-by: Klaus Jensen 
---
 hw/block/nvme.c | 7 ++-
 hw/block/nvme.h | 1 -
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index d60b19e1840f..a9d9a2912655 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -550,7 +550,8 @@ static void nvme_rw_cb(void *opaque, int ret)
 block_acct_failed(blk_get_stats(n->conf.blk), >acct);
 req->status = NVME_INTERNAL_DEV_ERROR;
 }
-if (req->has_sg) {
+
+if (req->qsg.nalloc) {
 qemu_sglist_destroy(>qsg);
 }
 nvme_enqueue_req_completion(cq, req);
@@ -559,7 +560,6 @@ static void nvme_rw_cb(void *opaque, int ret)
 static uint16_t nvme_flush(NvmeCtrl *n, NvmeNamespace *ns, NvmeCmd *cmd,
 NvmeRequest *req)
 {
-req->has_sg = false;
 block_acct_start(blk_get_stats(n->conf.blk), >acct, 0,
  BLOCK_ACCT_FLUSH);
 req->aiocb = blk_aio_flush(n->conf.blk, nvme_rw_cb, req);
@@ -585,7 +585,6 @@ static uint16_t nvme_write_zeros(NvmeCtrl *n, NvmeNamespace 
*ns, NvmeCmd *cmd,
 return NVME_LBA_RANGE | NVME_DNR;
 }
 
-req->has_sg = false;
 block_acct_start(blk_get_stats(n->conf.blk), >acct, 0,
  BLOCK_ACCT_WRITE);
 req->aiocb = blk_aio_pwrite_zeroes(n->conf.blk, offset, count,
@@ -623,7 +622,6 @@ static uint16_t nvme_rw(NvmeCtrl *n, NvmeNamespace *ns, 
NvmeCmd *cmd,
 }
 
 if (req->qsg.nsg > 0) {
-req->has_sg = true;
 block_acct_start(blk_get_stats(n->conf.blk), >acct, req->qsg.size,
  acct);
 req->aiocb = is_write ?
@@ -632,7 +630,6 @@ static uint16_t nvme_rw(NvmeCtrl *n, NvmeNamespace *ns, 
NvmeCmd *cmd,
 dma_blk_read(n->conf.blk, >qsg, data_offset, BDRV_SECTOR_SIZE,
  nvme_rw_cb, req);
 } else {
-req->has_sg = false;
 block_acct_start(blk_get_stats(n->conf.blk), >acct, req->iov.size,
  acct);
 req->aiocb = is_write ?
diff --git a/hw/block/nvme.h b/hw/block/nvme.h
index 0b6a8ae66559..5519b5cc7686 100644
--- a/hw/block/nvme.h
+++ b/hw/block/nvme.h
@@ -22,7 +22,6 @@ typedef struct NvmeRequest {
 struct NvmeSQueue   *sq;
 BlockAIOCB  *aiocb;
 uint16_tstatus;
-boolhas_sg;
 NvmeCqe cqe;
 BlockAcctCookie acct;
 QEMUSGList  qsg;
-- 
2.27.0