Re: [Qemu-block] [PATCH 3/3] block: Removed unused sector-based blocking I/O

[Eric Blake]

On 04/27/2018 09:01 AM, Alberto Garcia wrote:
> On Thu 26 Apr 2018 03:43:05 PM CEST, Eric Blake wrote:
>> We are gradually moving away from sector-based interfaces, towards
>> byte-based.  Now that all callers of blocking I/O have been converted
>> to use our preferred byte-based bdrv_p{read,write}(), we can
>> delete the unused bdrv_{read,write}().
>>
>> Signed-off-by: Eric Blake <address@hidden>
> 
> Reviewed-by: Alberto Garcia <address@hidden>
> 
>> -static int bdrv_rw_co(BdrvChild *child, int64_t sector_num, uint8_t *buf,
>> -                      int nb_sectors, bool is_write, BdrvRequestFlags flags)
>> -{
>> -    QEMUIOVector qiov;
>> -    struct iovec iov = {
>> -        .iov_base = (void *)buf,
>> -        .iov_len = nb_sectors * BDRV_SECTOR_SIZE,
>> -    };
>> -
>> -    if (nb_sectors < 0 || nb_sectors > BDRV_REQUEST_MAX_SECTORS) {
>> -        return -EINVAL;
>> -    }
> 
> Do we have a check for BDRV_REQUEST_MAX_BYTES in the byte-based API?

No, but we don't need one.  First, note that bs->bl.max_transfer is
currently uint32_t, so right now, no driver can set it any larger than
4G; although they can certainly set it smaller (for example, NBD sets it
to 32M).  Then note that bdrv_co_pwritev() and friends take care of
fragmenting any >4G request down to max_transfer.  The old check was to
ensure that a sector count couldn't overflow larger than a 4G byte count
(since the sector API had a 32-bit intrinsic limit on byte count, even
though the parameter was in sectors); but the public byte-based API
doesn't have an intrinsic limit on byte count, and takes care of
fragmenting down to any intrinsic limit for the driver callbacks.

But, having said that, you've made me wonder if it's time to increase
max_transfer to [u]int64_t, then audit ALL drivers to ensure that they
either properly handle requests >=4G or else set max_transfer <4G
(ideally, the block layer will default max_transfer to the value of
BDRV_REQUEST_MAX_BYTES, even if the audit means we no longer need that
macro).  Should be a separate series, though.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature