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 <anton.nefe...@virtuozzo.com> >> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> >> Reviewed-by: Alberto Garcia <be...@igalia.com> >> --- >> 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.