On 06/11/2016 08:15 PM, Fam Zheng wrote: > On Sat, 06/11 19:18, Eric Blake wrote: >> Commit e253f4b8 converted mirroring from sector-based bdrv_aio_* >> to byte-based blk_aio_*, but failed to account for the subtle >> difference in signatures (the former takes a semi-redundant length, >> the latter takes a flags parameter). Since all of our flags are >> currently smaller in size than BDRV_SECTOR_SIZE, it has no ill >> effects until we either perform sub-sector mirroring, or we start >> asserting that no unexpected flags are set. I found it while >> testing new asserts when qemu-iotests 132 started warning about an >> unknown flag 0x200000. >> >> Add an assert to help us catch any other improper flags. >> >> Signed-off-by: Eric Blake <[email protected]> >> --- >> block/block-backend.c | 2 ++ >> block/mirror.c | 6 ++---- >> 2 files changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/block/block-backend.c b/block/block-backend.c >> index 34500e6..a5dc6e3 100644 >> --- a/block/block-backend.c >> +++ b/block/block-backend.c >> @@ -945,6 +945,8 @@ static BlockAIOCB *blk_aio_prwv(BlockBackend *blk, >> int64_t offset, int bytes, >> acb->bh = NULL; >> acb->has_returned = false; >> >> + assert(flags <= 0x1f); >> + > > Maybe define BDRV_REQ_FLAGS_MAX in include/block/block.h so it's easier to > keep > this assertion valid when we introduce more flags in the future?
Oh, good idea. And I also don't know if blk_aio_prwv() is really the best place (it happened to be nicer for getting a good gdb backtrace of the culprit caller prior to entering coroutines, since coroutines tend to kill the call trace - but is too narrow in that it doesn't catch non-aio entry into the block layer) - so I should probably make the actual asserts live in better places like bdrv_driver_p*. I'll split the patch between bug fix and assertion additions, v2 coming up later today. > > Otherwise the patch looks good. > -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
