[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 21/23] block: Align block status requests
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v4 21/23] block: Align block status requests |
Date: |
Wed, 13 Sep 2017 15:36:21 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 |
On 09/13/2017 02:26 PM, Eric Blake wrote:
> On 09/13/2017 11:03 AM, Eric Blake wrote:
>> Any device that has request_alignment greater than 512 should be
>> unable to report status at a finer granularity; it may also be
>> simpler for such devices to be guaranteed that the block layer
>> has rounded things out to the granularity boundary (the way the
>> block layer already rounds all other I/O out). Besides, getting
>> the code correct for super-sector alignment also benefits us
>> for the fact that our public interface now has byte granularity,
>> even though none of our drivers have byte-level callbacks.
>>
>> Add an assertion in blkdebug that proves that the block layer
>> never requests status of unaligned sections, similar to what it
>> does on other requests (while still keeping the generic helper
>> in place for when future patches add a throttle driver). Note
>> that iotest 177 already covers this (it would fail if you use
>> just the blkdebug.c hunk without the io.c changes). Meanwhile,
>> we can drop assertions in callers that no longer have to pass
>> in sector-aligned addresses.
>
> Bummer - 'git bisect' says this patch causes iotests 190 to hang. I'm
> investigating root cause, but I'll have to post a fixup once I figure it
> out.
Found it:
> + /* Round out to request_alignment boundaries */
> + align = MAX(bs->bl.request_alignment, BDRV_SECTOR_SIZE);
> + aligned_offset = QEMU_ALIGN_DOWN(offset, align);
> + aligned_bytes = ROUND_UP(offset + bytes, align) - aligned_offset;
ROUND_UP(64-bit, 32-bit) has a subtle bug: it truncates the operation at
32 bits, instead of producing a 64-bit result. Using QEMU_ROUND_UP
instead does NOT have the bug.
That's a ticking time bomb, so I'll patch ROUND_UP() directly as a
pre-requisite, then reply to the cover letter with a Based-on tag.
--
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 v4 15/23] qemu-img: Add find_nonzero(), (continued)
- [Qemu-devel] [PATCH v4 15/23] qemu-img: Add find_nonzero(), Eric Blake, 2017/09/13
- [Qemu-devel] [PATCH v4 16/23] qemu-img: Drop redundant error message in compare, Eric Blake, 2017/09/13
- [Qemu-devel] [PATCH v4 13/23] qemu-img: Simplify logic in img_compare(), Eric Blake, 2017/09/13
- [Qemu-devel] [PATCH v4 12/23] block: Convert bdrv_get_block_status_above() to bytes, Eric Blake, 2017/09/13
- [Qemu-devel] [PATCH v4 17/23] qemu-img: Change check_empty_sectors() to byte-based, Eric Blake, 2017/09/13
- [Qemu-devel] [PATCH v4 18/23] qemu-img: Change compare_sectors() to be byte-based, Eric Blake, 2017/09/13
- [Qemu-devel] [PATCH v4 19/23] qemu-img: Change img_rebase() to be byte-based, Eric Blake, 2017/09/13
- [Qemu-devel] [PATCH v4 20/23] qemu-img: Change img_compare() to be byte-based, Eric Blake, 2017/09/13
- [Qemu-devel] [PATCH v4 21/23] block: Align block status requests, Eric Blake, 2017/09/13
- [Qemu-devel] [PATCH v4 22/23] block: Relax bdrv_aligned_preadv() assertion, Eric Blake, 2017/09/13
- [Qemu-devel] [PATCH v4 23/23] qemu-io: Relax 'alloc' now that block-status doesn't assert, Eric Blake, 2017/09/13
- Re: [Qemu-devel] [PATCH v4 00/23] make bdrv_get_block_status byte-based, Eric Blake, 2017/09/13