Re: [Qemu-block] [PATCH v2 5/8] scsi: move unmap error checking to the complete callback

[Alberto Garcia]

On Fri 19 Jan 2018 01:50:04 PM CET, Anton Nefedov wrote:
> This will help to account the operation in the following commit.
>
> The difference is that we don't call scsi_disk_req_check_error() before
> the 1st discard iteration anymore. That function also checks if
> the request is cancelled, however it shouldn't get canceled until it
> yields in blk_aio() functions anyway.
> Same approach is already used for emulate_write_same.
>
> Signed-off-by: Anton Nefedov 
> Reviewed-by: Vladimir Sementsov-Ogievskiy 

Reviewed-by: Alberto Garcia 

> @@ -1665,7 +1662,12 @@ static void scsi_unmap_complete(void *opaque, int ret)
>  r->req.aiocb = NULL;
>  
>  aio_context_acquire(blk_get_aio_context(s->qdev.conf.blk));
> -scsi_unmap_complete_noio(data, ret);
> +if (scsi_disk_req_check_error(r, ret, false)) {
> +scsi_req_unref(>req);
> +g_free(data);

It would be nice not to have the cleanup code duplicated here, but I
don't see any obvious alternative.

Berto

[Qemu-block] [PATCH v2 5/8] scsi: move unmap error checking to the complete callback

[Anton Nefedov]

This will help to account the operation in the following commit.

The difference is that we don't call scsi_disk_req_check_error() before
the 1st discard iteration anymore. That function also checks if
the request is cancelled, however it shouldn't get canceled until it
yields in blk_aio() functions anyway.
Same approach is already used for emulate_write_same.

Signed-off-by: Anton Nefedov 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
---
 hw/scsi/scsi-disk.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 7b8e0ed..693a754 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -1627,9 +1627,6 @@ static void scsi_unmap_complete_noio(UnmapCBData *data, 
int ret)
 SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
 
 assert(r->req.aiocb == NULL);
-if (scsi_disk_req_check_error(r, ret, false)) {
-goto done;
-}
 
 if (data->count > 0) {
 r->sector = ldq_be_p(>inbuf[0]);
@@ -1665,7 +1662,12 @@ static void scsi_unmap_complete(void *opaque, int ret)
 r->req.aiocb = NULL;
 
 aio_context_acquire(blk_get_aio_context(s->qdev.conf.blk));
-scsi_unmap_complete_noio(data, ret);
+if (scsi_disk_req_check_error(r, ret, false)) {
+scsi_req_unref(>req);
+g_free(data);
+} else {
+scsi_unmap_complete_noio(data, ret);
+}
 aio_context_release(blk_get_aio_context(s->qdev.conf.blk));
 }
 
-- 
2.7.4