qemu-block
[Top][All Lists]
Advanced

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

Re: [PATCH v2 2/6] block: truncate: Don't make backing file data visible


From: Eric Blake
Subject: Re: [PATCH v2 2/6] block: truncate: Don't make backing file data visible
Date: Wed, 20 Nov 2019 15:15:57 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.1.1

On 11/20/19 12:44 PM, 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 <address@hidden>
---
  block/io.c | 32 ++++++++++++++++++++++++++++++++
  1 file changed, 32 insertions(+)


+    if (new_bytes && bs->backing && prealloc == PREALLOC_MODE_OFF) {
+        int64_t backing_len;
+
+        backing_len = bdrv_getlength(backing_bs(bs));
+        if (backing_len < 0) {
+            ret = backing_len;
+            goto out;
+        }
+
+        if (backing_len > old_size) {
+            ret = bdrv_co_do_pwrite_zeroes(
+                    bs, old_size, MIN(new_bytes, backing_len - old_size),
+                    BDRV_REQ_ZERO_WRITE | BDRV_REQ_MAY_UNMAP);
+            if (ret < 0) {
+                goto out;
+            }
+        }
+    }

Note that if writing zeroes is not fast, and it turns out that we copy a lot of data rather than unallocated sections from the image being committed, that this can actually slow things down (doing a bulk pre-zero doubles up data I/O unless it is fast, which is why we added BDRV_REQ_NO_FALLBACK to avoid slow pre-zeroing). However, the complication of zeroing only the unallocated clusters rather than a bulk pre-zeroing for something that is an unlikely corner case (how often do you create an overlay shorter than the backing file?) is not worth the extra code maintenance (unlike in the 'qemu-img convert' case where it was worth the optimization). So I'm fine with how you fixed it here.

Reviewed-by: Eric Blake <address@hidden>

--
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]