[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 2/5] block: Uniform handling of 0-length bdrv
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v2 2/5] block: Uniform handling of 0-length bdrv_get_block_status() |
Date: |
Thu, 5 Oct 2017 09:41:59 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 |
On 10/05/2017 09:35 AM, Stefan Hajnoczi wrote:
> On Tue, Oct 03, 2017 at 08:43:44PM -0500, Eric Blake wrote:
>> Handle a 0-length block status request up front, with a uniform
>> return value claiming the area is not allocated.
>>
>> Most callers don't pass a length of 0 to bdrv_get_block_status()
>> and friends; but it definitely happens with a 0-length read when
>> copy-on-read is enabled. While we could audit all callers to
>> ensure that they never make a 0-length request, and then assert
>> that fact, it was just as easy to fix things to always report
>> success (as long as the callers are careful to not go into an
>> infinite loop). However, we had inconsistent behavior on whether
>> the status is reported as allocated or defers to the backing
>> layer, depending on what callbacks the driver implements, and
>> possibly wasting quite a few CPU cycles to get to that answer.
>> Consistently reporting unallocated up front doesn't really hurt
>> anything, and makes it easier both for callers (0-length requests
>> now have well-defined behavior) and for drivers (drivers don't
>> have to deal with 0-length requests).
>>
>> Signed-off-by: Eric Blake <address@hidden>
>>
> Reviewed-by: Stefan Hajnoczi <address@hidden>
>
> If you respin, please consider using a separate if statement to make the
> code clearer:
>
> if (!nb_sectors) {
> *pnum = 0;
> return 0;
> }
> if (sector_num >= total_sectors) {
> *pnum = 0;
> return BDRV_BLOCK_EOF;
> }
In fact, I think I would prefer to prioritize BDRV_BLOCK_EOF as the
first condition, rather than the second, by swapping those two
statements (that is, reading beyond EOF should set the EOF bit,
regardless of whether nb_sectors was non-zero).
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH v2 0/5] block: Avoid copy-on-read assertions, Eric Blake, 2017/10/03
- [Qemu-devel] [PATCH v2 1/5] qemu-io: Add -C for opening with copy-on-read, Eric Blake, 2017/10/03
- [Qemu-devel] [PATCH v2 2/5] block: Uniform handling of 0-length bdrv_get_block_status(), Eric Blake, 2017/10/03
- [Qemu-devel] [PATCH v2 3/5] block: Add blkdebug hook for copy-on-read, Eric Blake, 2017/10/03
- [Qemu-devel] [PATCH v2 4/5] block: Perform copy-on-read in loop, Eric Blake, 2017/10/03
- [Qemu-devel] [PATCH v2 5/5] iotests: Add test 197 for covering copy-on-read, Eric Blake, 2017/10/03
- Re: [Qemu-devel] [PATCH v2 0/5] block: Avoid copy-on-read assertions, no-reply, 2017/10/04