diff mbox series

[v5,7/9] scsi: account unmap operations

Message ID 20181031113418.29796-8-anton.nefedov@virtuozzo.com (mailing list archive)
State New, archived
Headers show
Series discard blockstats | expand

Commit Message

Anton Nefedov Oct. 31, 2018, 11:34 a.m. UTC
Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
---
 hw/scsi/scsi-disk.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Comments

Vladimir Sementsov-Ogievskiy Nov. 23, 2018, 6:25 p.m. UTC | #1
31.10.2018 14:34, Anton Nefedov wrote:
> Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>


Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

but be careful: on git am, the chunk about read-only case goes into scsi_disk_emulate_write_same instead of scsi_disk_emulate_unmap (at least for me, with latest master branch and git 2.18

> ---
>   hw/scsi/scsi-disk.c | 12 +++++++++++-
>   1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> index e132504913..dee71f9dde 100644
> --- a/hw/scsi/scsi-disk.c
> +++ b/hw/scsi/scsi-disk.c
> @@ -1663,10 +1663,16 @@ static void scsi_unmap_complete_noio(UnmapCBData *data, int ret)
>           r->sector = ldq_be_p(&data->inbuf[0]);
>           r->sector_count = ldl_be_p(&data->inbuf[8]) & 0xffffffffULL;
>           if (!check_lba_range(s, r->sector, r->sector_count)) {
> +            block_acct_invalid(blk_get_stats(s->qdev.conf.blk),
> +                               BLOCK_ACCT_UNMAP);
>               scsi_check_condition(r, SENSE_CODE(LBA_OUT_OF_RANGE));
>               goto done;
>           }
>   
> +        block_acct_start(blk_get_stats(s->qdev.conf.blk), &r->acct,
> +                         r->sector_count * s->qdev.blocksize,
> +                         BLOCK_ACCT_UNMAP);
> +
>           r->req.aiocb = blk_aio_pdiscard(s->qdev.conf.blk,
>                                           r->sector * s->qdev.blocksize,
>                                           r->sector_count * s->qdev.blocksize,
> @@ -1693,10 +1699,11 @@ static void scsi_unmap_complete(void *opaque, int ret)
>       r->req.aiocb = NULL;
>   
>       aio_context_acquire(blk_get_aio_context(s->qdev.conf.blk));
> -    if (scsi_disk_req_check_error(r, ret, false)) {
> +    if (scsi_disk_req_check_error(r, ret, true)) {
>           scsi_req_unref(&r->req);
>           g_free(data);
>       } else {
> +        block_acct_done(blk_get_stats(s->qdev.conf.blk), &r->acct);
>           scsi_unmap_complete_noio(data, ret);
>       }
>       aio_context_release(blk_get_aio_context(s->qdev.conf.blk));
> @@ -1728,6 +1735,7 @@ static void scsi_disk_emulate_unmap(SCSIDiskReq *r, uint8_t *inbuf)
>       }
>   
>       if (blk_is_read_only(s->qdev.conf.blk)) {
> +        block_acct_invalid(blk_get_stats(s->qdev.conf.blk), BLOCK_ACCT_UNMAP);


this one goes to scsi_disk_emulate_write_same on automatic patch apply.


>           scsi_check_condition(r, SENSE_CODE(WRITE_PROTECTED));
>           return;
>       }
> @@ -1743,10 +1751,12 @@ static void scsi_disk_emulate_unmap(SCSIDiskReq *r, uint8_t *inbuf)
>       return;
>   
>   invalid_param_len:
> +    block_acct_invalid(blk_get_stats(s->qdev.conf.blk), BLOCK_ACCT_UNMAP);
>       scsi_check_condition(r, SENSE_CODE(INVALID_PARAM_LEN));
>       return;
>   
>   invalid_field:
> +    block_acct_invalid(blk_get_stats(s->qdev.conf.blk), BLOCK_ACCT_UNMAP);
>       scsi_check_condition(r, SENSE_CODE(INVALID_FIELD));
>   }
>   
>
Anton Nefedov Nov. 26, 2018, 12:20 p.m. UTC | #2
On 23/11/2018 9:25 PM, Vladimir Sementsov-Ogievskiy wrote:
> 31.10.2018 14:34, Anton Nefedov wrote:
>> Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
> 
> 
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> 
> but be careful: on git am, the chunk about read-only case goes into scsi_disk_emulate_write_same instead of scsi_disk_emulate_unmap (at least for me, with latest master branch and git 2.18
> 

It does. That's weird :/
Thanks!
diff mbox series

Patch

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index e132504913..dee71f9dde 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -1663,10 +1663,16 @@  static void scsi_unmap_complete_noio(UnmapCBData *data, int ret)
         r->sector = ldq_be_p(&data->inbuf[0]);
         r->sector_count = ldl_be_p(&data->inbuf[8]) & 0xffffffffULL;
         if (!check_lba_range(s, r->sector, r->sector_count)) {
+            block_acct_invalid(blk_get_stats(s->qdev.conf.blk),
+                               BLOCK_ACCT_UNMAP);
             scsi_check_condition(r, SENSE_CODE(LBA_OUT_OF_RANGE));
             goto done;
         }
 
+        block_acct_start(blk_get_stats(s->qdev.conf.blk), &r->acct,
+                         r->sector_count * s->qdev.blocksize,
+                         BLOCK_ACCT_UNMAP);
+
         r->req.aiocb = blk_aio_pdiscard(s->qdev.conf.blk,
                                         r->sector * s->qdev.blocksize,
                                         r->sector_count * s->qdev.blocksize,
@@ -1693,10 +1699,11 @@  static void scsi_unmap_complete(void *opaque, int ret)
     r->req.aiocb = NULL;
 
     aio_context_acquire(blk_get_aio_context(s->qdev.conf.blk));
-    if (scsi_disk_req_check_error(r, ret, false)) {
+    if (scsi_disk_req_check_error(r, ret, true)) {
         scsi_req_unref(&r->req);
         g_free(data);
     } else {
+        block_acct_done(blk_get_stats(s->qdev.conf.blk), &r->acct);
         scsi_unmap_complete_noio(data, ret);
     }
     aio_context_release(blk_get_aio_context(s->qdev.conf.blk));
@@ -1728,6 +1735,7 @@  static void scsi_disk_emulate_unmap(SCSIDiskReq *r, uint8_t *inbuf)
     }
 
     if (blk_is_read_only(s->qdev.conf.blk)) {
+        block_acct_invalid(blk_get_stats(s->qdev.conf.blk), BLOCK_ACCT_UNMAP);
         scsi_check_condition(r, SENSE_CODE(WRITE_PROTECTED));
         return;
     }
@@ -1743,10 +1751,12 @@  static void scsi_disk_emulate_unmap(SCSIDiskReq *r, uint8_t *inbuf)
     return;
 
 invalid_param_len:
+    block_acct_invalid(blk_get_stats(s->qdev.conf.blk), BLOCK_ACCT_UNMAP);
     scsi_check_condition(r, SENSE_CODE(INVALID_PARAM_LEN));
     return;
 
 invalid_field:
+    block_acct_invalid(blk_get_stats(s->qdev.conf.blk), BLOCK_ACCT_UNMAP);
     scsi_check_condition(r, SENSE_CODE(INVALID_FIELD));
 }