On Nov 25 23:11, Minwoo Im wrote: > Hello, > > On 20-11-24 08:37:14, Klaus Jensen wrote: > > From: Gollu Appalanaidu <anaidu.go...@samsung.com> > > > > Add the Compare command. > > > > This implementation uses a bounce buffer to read in the data from > > storage and then compare with the host supplied buffer. > > > > Signed-off-by: Gollu Appalanaidu <anaidu.go...@samsung.com> > > [k.jensen: rebased] > > Signed-off-by: Klaus Jensen <k.jen...@samsung.com> > > --- > > hw/block/nvme.c | 100 +++++++++++++++++++++++++++++++++++++++++- > > hw/block/trace-events | 2 + > > 2 files changed, 101 insertions(+), 1 deletion(-) > > > > diff --git a/hw/block/nvme.c b/hw/block/nvme.c > > index f7f888402b06..f88710ca3948 100644 > > --- a/hw/block/nvme.c > > +++ b/hw/block/nvme.c > > @@ -999,6 +999,50 @@ static void nvme_aio_discard_cb(void *opaque, int ret) > > nvme_enqueue_req_completion(nvme_cq(req), req); > > } > > > > +struct nvme_compare_ctx { > > + QEMUIOVector iov; > > + uint8_t *bounce; > > + size_t len; > > +}; > > + > > +static void nvme_compare_cb(void *opaque, int ret) > > +{ > > + NvmeRequest *req = opaque; > > + NvmeNamespace *ns = req->ns; > > + struct nvme_compare_ctx *ctx = req->opaque; > > + g_autofree uint8_t *buf = NULL; > > nit-picking here: unnecessary initialization to NULL. >
I don't think it is unnecessary when it using g_autofree? > > + uint16_t status; > > + > > + trace_pci_nvme_compare_cb(nvme_cid(req)); > > + > > + if (!ret) { > > + block_acct_done(blk_get_stats(ns->blkconf.blk), &req->acct); > > + } else { > > + block_acct_failed(blk_get_stats(ns->blkconf.blk), &req->acct); > > + nvme_aio_err(req, ret); > > + goto out; > > + } > > + > > + buf = g_malloc(ctx->len); > > + > > + status = nvme_dma(nvme_ctrl(req), buf, ctx->len, > > DMA_DIRECTION_TO_DEVICE, > > + req); > > + if (status) { > > + goto out; > > + } > > Don't we need to give status value to req->status in case of > (status != 0)? If we don't give it to req->status, it will complete > with success status code even it fails during the nvme_dma(). > Nice catch! nvme_aio_err normally takes care of this for blk/aio errors, but this one slipped. Thanks!
signature.asc
Description: PGP signature