Re: [Qemu-devel] [PATCH v7 06/19] scsi-disk: Switch to byte-based aio block access

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1777 bytes --]

On 05/10/2016 02:55 AM, Kevin Wolf wrote:
> Am 06.05.2016 um 18:26 hat Eric Blake geschrieben:
>> Sector-based blk_aio_readv() and blk_aio_writev() should die; switch
>> to byte-based blk_aio_preadv() and blk_aio_pwritev() instead.
>>
>> As part of the cleanup, scsi_init_iovec() no longer needs to return
>> a value, and reword a comment.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>>

>> @@ -118,7 +118,6 @@ static uint32_t scsi_init_iovec(SCSIDiskReq *r, size_t size)
>>      }
>>      r->iov.iov_len = MIN(r->sector_count * 512, r->buflen);
>>      qemu_iovec_init_external(&r->qiov, &r->iov, 1);
>> -    return r->qiov.size / 512;
> 
> The return value was MIN(r->sector_count, SCSI_DMA_BUF_SIZE / 512).
> 
>>  }
>>
>>  static void scsi_disk_save_request(QEMUFile *f, SCSIRequest *req)

>> -        n = scsi_init_iovec(r, SCSI_DMA_BUF_SIZE);
>> +        scsi_init_iovec(r, SCSI_DMA_BUF_SIZE);
>>          block_acct_start(blk_get_stats(s->qdev.conf.blk), &r->acct,
>> -                         n * BDRV_SECTOR_SIZE, BLOCK_ACCT_READ);
>> -        r->req.aiocb = blk_aio_readv(s->qdev.conf.blk, r->sector, &r->qiov, n,
>> -                                     scsi_read_complete, r);
>> +                         SCSI_DMA_BUF_SIZE, BLOCK_ACCT_READ);
> 
> But here you use SCSI_DMA_BUF_SIZE without considering r->sector_count.
> Is this correct or are requests that are smaller than SCSI_DMA_BUF_SIZE
> now accounted for more data than they actually read?
> 
> Would r->qiov.size be more obviously correct? You did that for writes.

Yes. Is that something you are able to fix on commit, or should I submit
a fixup?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]