On 23.04.20 17:01, Kevin Wolf wrote: > When extending the size of an image that has a backing file larger than > its old size, make sure that the backing file data doesn't become > visible in the guest, but the added area is properly zeroed out. > > Consider the following scenario where the overlay is shorter than its > backing file: > > base.qcow2: AAAAAAAA > overlay.qcow2: BBBB > > When resizing (extending) overlay.qcow2, the new blocks should not stay > unallocated and make the additional As from base.qcow2 visible like > before this patch, but zeros should be read. > > A similar case happens with the various variants of a commit job when an > intermediate file is short (- for unallocated): > > base.qcow2: A-A-AAAA > mid.qcow2: BB-B > top.qcow2: C--C--C- > > After commit top.qcow2 to mid.qcow2, the following happens: > > mid.qcow2: CB-C00C0 (correct result) > mid.qcow2: CB-C--C- (before this fix) > > Without the fix, blocks that previously read as zeros on top.qcow2 > suddenly turn into A. > > Signed-off-by: Kevin Wolf <kw...@redhat.com> > Reviewed-by: Alberto Garcia <be...@igalia.com>
Seems like Berto gave you a rather broad R-b in v4. :) > --- > block/io.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/block/io.c b/block/io.c > index 795075954e..f618db3499 100644 > --- a/block/io.c > +++ b/block/io.c > @@ -3394,6 +3394,30 @@ int coroutine_fn bdrv_co_truncate(BdrvChild *child, > int64_t offset, bool exact, > goto out; > } > > + /* > + * If the image has a backing file that is large enough that it would > + * provide data for the new area, we cannot leave it unallocated because > + * then the backing file content would become visible. Instead, zero-fill > + * the new area. > + * > + * Note that if the image has a backing file, but was opened without the > + * backing file, taking care of keeping things consistent with that > backing > + * file is the user's responsibility. > + */ > + if (new_bytes && bs->backing) { > + int64_t backing_len; > + > + backing_len = bdrv_getlength(backing_bs(bs)); > + if (backing_len < 0) { > + ret = backing_len; Shouldn’t we set errp? Max > + goto out; > + } > + > + if (backing_len > old_size) { > + flags |= BDRV_REQ_ZERO_WRITE; > + } > + } > + > if (drv->bdrv_co_truncate) { > if (flags & ~bs->supported_truncate_flags) { > error_setg(errp, "Block driver does not support requested > flags"); >
signature.asc
Description: OpenPGP digital signature