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