On 14.07.20 20:37, Andrey Shinkevich wrote: > On 25.06.2020 18:21, Max Reitz wrote: >> Instead of looking at just bs->file and bs->backing, we should look at >> all children that could end up receiving forwarded requests. >> >> Signed-off-by: Max Reitz <mre...@redhat.com> >> --- >> block/io.c | 32 ++++++++++++++++---------------- >> 1 file changed, 16 insertions(+), 16 deletions(-) >> >> diff --git a/block/io.c b/block/io.c >> index c2af7711d6..37057f13e0 100644 >> --- a/block/io.c >> +++ b/block/io.c >> @@ -135,6 +135,8 @@ static void bdrv_merge_limits(BlockLimits *dst, >> const BlockLimits *src) >> void bdrv_refresh_limits(BlockDriverState *bs, Error **errp) >> { >> BlockDriver *drv = bs->drv; >> + BdrvChild *c; >> + bool have_limits; >> Error *local_err = NULL; >> memset(&bs->bl, 0, sizeof(bs->bl)); >> @@ -149,14 +151,21 @@ void bdrv_refresh_limits(BlockDriverState *bs, >> Error **errp) >> drv->bdrv_co_preadv_part) ? 1 : 512; >> /* Take some limits from the children as a default */ >> - if (bs->file) { >> - bdrv_refresh_limits(bs->file->bs, &local_err); >> - if (local_err) { >> - error_propagate(errp, local_err); >> - return; >> + have_limits = false; >> + QLIST_FOREACH(c, &bs->children, next) { >> + if (c->role & (BDRV_CHILD_DATA | BDRV_CHILD_FILTERED | >> BDRV_CHILD_COW)) >> + { >> + bdrv_refresh_limits(c->bs, &local_err); >> + if (local_err) { >> + error_propagate(errp, local_err); >> + return; >> + } >> + bdrv_merge_limits(&bs->bl, &c->bs->bl); >> + have_limits = true; >> } >> - bdrv_merge_limits(&bs->bl, &bs->file->bs->bl); >> - } else { >> + } >> + >> + if (!have_limits) { > > > This conditioned piece of code worked with (bs->file == NULL) only. > > Now, it works only if there are neither bs->file, nor bs->backing, nor > else filtered children. > > Is it OK and doesn't break the logic for all cases?
Hm. Good question. I think the answer is it’s OK. For DATA and FILTERED, it makes absolute sense to just use their alignments. For COW, maybe not so much? But if there’s a COW child, there has to be a DATA child as well (in practice). So we’ll always consider its alignment, too. (And hypothetically speaking, if there was a COW child but no DATA child, then the only alignment we need to observe is in fact the one of the COW child.) Max
signature.asc
Description: OpenPGP digital signature