[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 09/17] block: Refactor bdrv_has_zero_init{,_truncate}

From: Eric Blake
Subject: Re: [PATCH 09/17] block: Refactor bdrv_has_zero_init{,_truncate}
Date: Wed, 5 Feb 2020 08:36:26 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1

On 2/5/20 8:25 AM, Vladimir Sementsov-Ogievskiy wrote:

But when you truncate them (with PREALLOC_MODE_OFF, as
BlockDriver.bdrv_has_zero_init_truncate()’s comment explains), the new
area is always going to be 0, regardless of initial preallocation.

ah yes, due to qcow2 zero clusters.

Hmm. Do we actually set the zero flag on unallocated clusters when resizing a qcow2 image?  That would be an O(n) operation (we have to visit the L2 entry for each added cluster, even if only to set the zero cluster bit).  Or do we instead just rely on the fact that qcow2 is inherently sparse, and that when you resize the guest-visible size without writing any new clusters, then it is only subsequent guest access to those addresses that finally allocate clusters, making resize O(1) (update the qcow2 metadata cluster, but not any L2 tables) while still reading 0 from the new data.  To some extent, that's what the allocation mode is supposed to control.

We must mark as ZERO new cluster at least if there is a _larger_ backing file, to prevent data from backing file become available for the guest. But we don't do it. It's a bug and there is fixing series from Kevin, I've just pinged it:
"[PATCH for-4.2? v3 0/8] block: Fix resize (extending) of short overlays"

There's a difference for a backing file larger than the qcow2 file, and the protocol layer larger than the qcow2 file. Visually, with the following four nodes:

f1 [qcow2 format] <- f2 [qcow2 format]
  v                        v
p1 [file protocol]    p2 [file protocol]

If f1 is larger than f2, then resizing f2 without writing zero clusters leaks the data from f1 into f2. The block layer knows this: prior to this series, bdrv_has_zero_init_truncate() returns 0 if bs->backing is present; and even in this series, see patch 6/17 which continues to force a 0 return rather than calling into the driver if the sizes are suspect. But that is an uncommon corner case; in short, the qcow2 callback .bdrv_has_zero_init_truncate is NOT reachable in that scenario, whether before or after this series.

On the other hand, if p2 is larger than f2, resizing f2 reads zeroes. That's because qcow2 HAS to add L2 mappings into p2 before data from p2 can leak, but .bdrv_co_truncate(PREALLOC_MODE_OFF) does not add any L2 mappings. Thus, qcow2 blindly returning 1 for .bdrv_has_zero_init_truncate was correct (other than the anomaly of bs->encrypted, also fixed earlier in this series).

Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

reply via email to

[Prev in Thread] Current Thread [Next in Thread]