[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 6/8] scsi: account unmap operations
From: |
Anton Nefedov |
Subject: |
Re: [Qemu-devel] [PATCH v4 6/8] scsi: account unmap operations |
Date: |
Mon, 8 Oct 2018 14:43:19 +0000 |
On 4/10/2018 6:47 PM, Kevin Wolf wrote:
> Am 21.08.2018 um 11:46 hat Anton Nefedov geschrieben:
>> Signed-off-by: Anton Nefedov <address@hidden>
>> Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>> Reviewed-by: Alberto Garcia <address@hidden>
>> ---
>> hw/scsi/scsi-disk.c | 9 ++++++++-
>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
>> index 9d10daf..0aac137 100644
>> --- a/hw/scsi/scsi-disk.c
>> +++ b/hw/scsi/scsi-disk.c
>> @@ -1664,6 +1664,10 @@ static void scsi_unmap_complete_noio(UnmapCBData
>> *data, int ret)
>> goto done;
>> }
>>
>> + block_acct_start(blk_get_stats(s->qdev.conf.blk), &r->acct,
>> + r->sector_count * s->qdev.blocksize,
>> + BLOCK_ACCT_UNMAP);
>
> If the check just above this (check_lba_range) fails, we should account
> for an invalid request.
>
Done.
>> r->req.aiocb = blk_aio_pdiscard(s->qdev.conf.blk,
>> r->sector * s->qdev.blocksize,
>> r->sector_count *
>> s->qdev.blocksize,
>> @@ -1690,10 +1694,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));
>> @@ -1740,10 +1745,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));
>> }
>
> What about the blk_is_read_only() case which directly returns without
> jumping to one of the error labels?
>
So basically anything we don't bring to blk layer is invalid req.
Fixed.