On Mon, Jul 03, 2017 at 12:51:45PM -0500, Eric Blake wrote:
On 07/03/2017 11:12 AM, Eric Blake wrote:On 06/29/2017 01:43 PM, Manos Pitsidianakis wrote:bdrv_co_get_block_status_from_file() and bdrv_co_get_block_status_from_backing() set *file to bs->file and bs->backing respectively, so that bdrv_co_get_block_status() can recurse to them. Future block drivers won't have to duplicate code to implement this.Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com> Signed-off-by: Manos Pitsidianakis <el13...@mail.ntua.gr> --- block/blkdebug.c | 12 +----------- block/commit.c | 12 +----------- block/io.c | 24 ++++++++++++++++++++++++ block/mirror.c | 12 +----------- include/block/block_int.h | 16 ++++++++++++++++ 5 files changed, 43 insertions(+), 33 deletions(-)My [still-needs-rebasing] part 3 series converting bdrv_co_get_block_status to byte-based will be impacted by this, but I'd rather yours go in first and I can deal with the rebase fallout.In fact, my rebase fallout is to completely delete bdrv_co_get_block_status_from_file(). Why? Because blkdebug is the only client, but I want to enhance blkdebug to add in-place assert()ions that the values passed by the block layer are properly aligned to bs->bl.request_alignment (matching what blkdebug does for read and write - after all, if a device cannot access smaller than a given alignment for a read, it should not be able to report different statuses half-way through that granularity). However, in-place assertions are no longer generic, so we no longer have a client of a generic helper function referring to bs->file. Which means you are REALLY left only with commit and mirror as the twofunctions that have a common implementation of pointing back to backing.
Hello Eric, thanks for taking the time to review my patches.The throttle driver I'm working on passes bdrv_co_get_block_status() to bs->file. If there is a problem with an unused default function (it's not static so will compile, but it might not be up to standard), you can just remove it and I will reintroduce it when it's needed. CC me on those patches when you send them if you can.
signature.asc
Description: PGP signature
