I started looking into adding tests, but it doesn't look like blkdebug supports changing the blocksize. I saw that it supports changing the memory alignment to 4k via the align parameter, but it doesn't implement bdrv_probe_blocksizes so the blocksize stays at 512.
Would it be worth adding a blocksize parameter to blkdebug? (or maybe log-blksize & phys-blksize?) Or is there another way to set the blocksize of a qdev? Thanks, Kit On Fri, May 21, 2021 at 11:20 AM Paolo Bonzini <pbonz...@redhat.com> wrote: > On 21/05/21 16:28, Kit Westneat wrote: > > check_lba_range expects sectors to be expressed in original qdev > blocksize, but > > scsi_unmap_complete_noio was translating them to 512 block sizes, which > was > > causing sense errors in the larger LBAs in devices using a 4k block size. > > > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/345 > > Signed-off-by: Kit Westneat <kit.westn...@gmail.com> > > --- > > hw/scsi/scsi-disk.c | 12 +++++++----- > > 1 file changed, 7 insertions(+), 5 deletions(-) > > > > diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c > > index 3580e7ee61..e8a547dbb7 100644 > > --- a/hw/scsi/scsi-disk.c > > +++ b/hw/scsi/scsi-disk.c > > @@ -1582,6 +1582,7 @@ invalid_field: > > scsi_check_condition(r, SENSE_CODE(INVALID_FIELD)); > > } > > > > +/* sector_num and nb_sectors expected to be in qdev blocksize */ > > static inline bool check_lba_range(SCSIDiskState *s, > > uint64_t sector_num, uint32_t > nb_sectors) > > { > > @@ -1614,11 +1615,12 @@ static void scsi_unmap_complete_noio(UnmapCBData > *data, int ret) > > assert(r->req.aiocb == NULL); > > > > if (data->count > 0) { > > - r->sector = ldq_be_p(&data->inbuf[0]) > > - * (s->qdev.blocksize / BDRV_SECTOR_SIZE); > > - r->sector_count = (ldl_be_p(&data->inbuf[8]) & 0xffffffffULL) > > - * (s->qdev.blocksize / BDRV_SECTOR_SIZE); > > - if (!check_lba_range(s, r->sector, r->sector_count)) { > > + uint64_t sector_num = ldq_be_p(&data->inbuf[0]); > > + uint32_t nb_sectors = ldl_be_p(&data->inbuf[8]) & 0xffffffffULL; > > + r->sector = sector_num * (s->qdev.blocksize / BDRV_SECTOR_SIZE); > > + r->sector_count = nb_sectors * (s->qdev.blocksize / > BDRV_SECTOR_SIZE); > > + > > + if (!check_lba_range(s, sector_num, nb_sectors)) { > > block_acct_invalid(blk_get_stats(s->qdev.conf.blk), > > BLOCK_ACCT_UNMAP); > > scsi_check_condition(r, SENSE_CODE(LBA_OUT_OF_RANGE)); > > > > Queued, thanks! > > If you want to produce a testcase for tests/qtest/virtio-scsi-test.c, I > won't complain. > > Paolo > >
