On 07/04/2017 04:44 AM, Fam Zheng wrote: > On Mon, 07/03 17:14, 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 >> note that iotest 177 already covers this (it would fail if you > > Drop one "note"?
Indeed. There are studies on how people read that show that redundant
words that cross a line boundaries are the most easy to mentally overlook.
>> + /* 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;
>> +
>> + {
>
> Not sure why using a {} block here?
>
>> + int count; /* sectors */
Because it makes it easier (less indentation churn) to delete the code
when series 4 later deletes the .bdrv_co_get_block_status sector-based
callback in favor of the newer .bdrv_co_block_status byte-based (patch
16/15 which start series 4 turns it into an if statement, then a later
patch deletes the entire conditional); it is also justifiable because it
creates a tighter scope for 'int count' which is needed only on the
boundary of sector-based operations when the rest of the function is
byte-based (rather than declaring the helper variable up front). I'll
have to call it out more specifically in the commit message as intentional.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
