On 21.11.19 12:34, Kevin Wolf wrote: > Am 21.11.2019 um 09:59 hat Max Reitz geschrieben: >> On 20.11.19 19:44, 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> >>> --- >>> block/io.c | 32 ++++++++++++++++++++++++++++++++ >>> 1 file changed, 32 insertions(+) >> >> Zeroing the intersection may take some time. So is it right for QMP’s >> block_resize to do this, seeing it is a synchronous operation? > > What else would be right? Returning an error?
Going through a deprecation cycle. > Common cases (raw and qcow2 v3 without external data files) are quick > anyway. Well, but quick enough for a fully blocking operation? >> As far as I can tell, jobs actually have the same problem. I don’t >> think mirror or commit have a pause point before truncating, so they >> still block the monitor there, don’t they? > > Do you really need a pause point? They call bdrv_co_truncate() from > inside the job coroutine, so it will yield. I would expect that this > is enough. OK then. > But in fact, all jobs have a pause point before even calling .run(), so > even if that made a difference, it should still be fine. Good. But I believe this is still a problem for block_resize. I don’t see why this needs to be fixed in 4.2-rc3. What’s the problem with going through a proper deprecation cycle other than the fact that we can’t start it in 4.2 because we don’t have a resize job yet? Max
signature.asc
Description: OpenPGP digital signature