[PULL 00/30] Block layer patches

[PULL 00/30] Block layer patches

[Kevin Wolf]

The following changes since commit 1b8c45899715d292398152ba97ef755ccaf84680:

  Merge remote-tracking branch 'remotes/dgilbert/tags/pull-migration-20200507a' into staging (2020-05-07 18:43:20 +0100)

are available in the Git repository at:

  git://repo.or.cz/qemu/kevin.git tags/for-upstream

for you to fetch changes up to 47e0b38a13935cb666f88964c3096654092f42d6:

  block: Drop unused .bdrv_has_zero_init_truncate (2020-05-08 13:26:35 +0200)

----------------------------------------------------------------
Block layer patches:

- qcow2: Fix preallocation on block devices
- backup: Make sure that source and target size match
- vmdk: Fix zero cluster handling
- Follow-up cleanups and fixes for the truncate changes
- iotests: Skip more tests if required drivers are missing

----------------------------------------------------------------
Alberto Garcia (1):
      qcow2: Avoid integer wraparound in qcow2_co_truncate()

Eric Blake (9):
      gluster: Drop useless has_zero_init callback
      file-win32: Support BDRV_REQ_ZERO_WRITE for truncate
      nfs: Support BDRV_REQ_ZERO_WRITE for truncate
      rbd: Support BDRV_REQ_ZERO_WRITE for truncate
      sheepdog: Support BDRV_REQ_ZERO_WRITE for truncate
      ssh: Support BDRV_REQ_ZERO_WRITE for truncate
      parallels: Rework truncation logic
      vhdx: Rework truncation logic
      block: Drop unused .bdrv_has_zero_init_truncate

Kevin Wolf (11):
      vmdk: Rename VmdkMetaData.valid to new_allocation
      vmdk: Fix zero cluster allocation
      vmdk: Fix partial overwrite of zero cluster
      vmdk: Don't update L2 table for zero write on zero cluster
      vmdk: Flush only once in vmdk_L2update()
      iotests: vmdk: Enable zeroed_grained=on by default
      iotests/283: Use consistent size for source and target
      backup: Improve error for bdrv_getlength() failure
      backup: Make sure that source and target size match
      iotests: Backup with different source/target size
      iotests/055: Use cache.no-flush for vmdk target

Max Reitz (1):
      qcow2: Fix preallocation on block devices

Vladimir Sementsov-Ogievskiy (8):
      iotests: handle tmpfs
      iotests/082: require bochs
      iotests/148: use skip_if_unsupported
      iotests/041: drop self.assert_no_active_block_jobs()
      iotests/055: refactor compressed backup to vmdk
      iotests/055: skip vmdk target tests if vmdk is not whitelisted
      iotests/109: mark required formats as required to support whitelisting
      iotests/113: mark bochs as required to support whitelisting

 include/block/block.h        |   1 -
 include/block/block_int.h    |   7 ---
 block.c                      |  21 --------
 block/backup-top.c           |  14 +++--
 block/backup.c               |  18 +++++--
 block/file-posix.c           |   1 -
 block/file-win32.c           |   4 +-
 block/gluster.c              |  14 -----
 block/nfs.c                  |   4 +-
 block/parallels.c            |  25 +++++----
 block/qcow2.c                |  23 ++++++---
 block/qed.c                  |   1 -
 block/raw-format.c           |   6 ---
 block/rbd.c                  |   4 +-
 block/sheepdog.c             |   4 +-
 block/ssh.c                  |   5 +-
 block/vhdx.c                 |  89 ++++++++++++++++++--------------
 block/vmdk.c                 |  47 ++++++++++-------
 tests/qemu-iotests/041       |   8 ---
 tests/qemu-iotests/055       | 120 ++++++++++++++++++++++++++++++-------------
 tests/qemu-iotests/055.out   |   4 +-
 tests/qemu-iotests/059       |   6 +--
 tests/qemu-iotests/082       |   1 +
 tests/qemu-iotests/091       |   2 +-
 tests/qemu-iotests/109       |   1 +
 tests/qemu-iotests/113       |   4 +-
 tests/qemu-iotests/148       |   1 +
 tests/qemu-iotests/283       |   6 ++-
 tests/qemu-iotests/283.out   |   2 +-
 tests/qemu-iotests/292       |  73 ++++++++++++++++++++++++++
 tests/qemu-iotests/292.out   |  24 +++++++++
 tests/qemu-iotests/check     |   3 ++
 tests/qemu-iotests/common.rc |  37 ++++++++++++-
 tests/qemu-iotests/group     |   1 +
 34 files changed, 386 insertions(+), 195 deletions(-)
 create mode 100755 tests/qemu-iotests/292
 create mode 100644 tests/qemu-iotests/292.out

[PULL 01/30] iotests: handle tmpfs

[Kevin Wolf]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Some tests requires O_DIRECT, or want it by default. Introduce smarter
O_DIRECT handling:

- Check O_DIRECT in common.rc, if it is requested by selected
cache-mode.

- Support second fall-through argument in _default_cache_mode

Inspired-by: Max's 23e1d054112cec1e
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200430124713.3067-2-vsementsov@virtuozzo.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/091       |  2 +-
 tests/qemu-iotests/common.rc | 37 ++++++++++++++++++++++++++++++++++--
 2 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/091 b/tests/qemu-iotests/091
index d2a2aca347..68fbfd777b 100755
--- a/tests/qemu-iotests/091
+++ b/tests/qemu-iotests/091
@@ -46,8 +46,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 _supported_fmt qcow2
 _supported_proto file
 _supported_os Linux
-_default_cache_mode none
 _supported_cache_modes writethrough none writeback
+_default_cache_mode none writeback
 
 size=1G
 
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index bf3b9fdea0..ba912555ca 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -673,11 +673,44 @@ _supported_cache_modes()
     _notrun "not suitable for cache mode: $CACHEMODE"
 }
 
+# Check whether the filesystem supports O_DIRECT
+_check_o_direct()
+{
+    $QEMU_IMG create -f raw "$TEST_IMG".test_o_direct 1M > /dev/null
+    out=$($QEMU_IO -f raw -t none -c quit "$TEST_IMG".test_o_direct 2>&1)
+    rm -f "$TEST_IMG".test_o_direct
+
+    [[ "$out" != *"O_DIRECT"* ]]
+}
+
+_require_o_direct()
+{
+    if ! _check_o_direct; then
+        _notrun "file system on $TEST_DIR does not support O_DIRECT"
+    fi
+}
+
+_check_cache_mode()
+{
+    if [ $CACHEMODE == "none" ] || [ $CACHEMODE == "directsync" ]; then
+        _require_o_direct
+    fi
+}
+
+_check_cache_mode
+
+# $1 - cache mode to use by default
+# $2 - (optional) cache mode to use by default if O_DIRECT is not supported
 _default_cache_mode()
 {
     if $CACHEMODE_IS_DEFAULT; then
-        CACHEMODE="$1"
-        QEMU_IO="$QEMU_IO --cache $1"
+        if [ -z "$2" ] || _check_o_direct; then
+            CACHEMODE="$1"
+        else
+            CACHEMODE="$2"
+        fi
+        QEMU_IO="$QEMU_IO --cache $CACHEMODE"
+        _check_cache_mode
         return
     fi
 }
-- 
2.25.3

[PULL 02/30] iotests/082: require bochs

[Kevin Wolf]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Test fails if bochs not whitelisted, so, skip it in this case.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200430124713.3067-3-vsementsov@virtuozzo.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/082 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/qemu-iotests/082 b/tests/qemu-iotests/082
index 3286c2c6db..1998965ed4 100755
--- a/tests/qemu-iotests/082
+++ b/tests/qemu-iotests/082
@@ -38,6 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _supported_fmt qcow2
 _supported_proto file nfs
+_require_drivers bochs
 
 run_qemu_img()
 {
-- 
2.25.3

[PULL 03/30] iotests/148: use skip_if_unsupported

[Kevin Wolf]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Skip test-case with quorum if quorum is not whitelisted.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200430124713.3067-4-vsementsov@virtuozzo.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/148 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/qemu-iotests/148 b/tests/qemu-iotests/148
index 90931948e3..5e14a455b1 100755
--- a/tests/qemu-iotests/148
+++ b/tests/qemu-iotests/148
@@ -47,6 +47,7 @@ sector = "%d"
 ''' % bad_sector)
         file.close()
 
+    @iotests.skip_if_unsupported(['quorum'])
     def setUp(self):
         driveopts = ['driver=quorum', 'vote-threshold=2']
         driveopts.append('read-pattern=%s' % self.read_pattern)
-- 
2.25.3

[PULL 04/30] iotests/041: drop self.assert_no_active_block_jobs()

[Kevin Wolf]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Drop check for no block-jobs: it's obvious that there no jobs
immediately after vm.launch().

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200430124713.3067-5-vsementsov@virtuozzo.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/041 | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
index 46bf1f6c81..1812dd8479 100755
--- a/tests/qemu-iotests/041
+++ b/tests/qemu-iotests/041
@@ -904,8 +904,6 @@ class TestRepairQuorum(iotests.QMPTestCase):
                 pass
 
     def test_complete(self):
-        self.assert_no_active_block_jobs()
-
         result = self.vm.qmp('drive-mirror', job_id='job0', device='quorum0',
                              sync='full', node_name="repair0", replaces="img1",
                              target=quorum_repair_img, format=iotests.imgfmt)
@@ -919,8 +917,6 @@ class TestRepairQuorum(iotests.QMPTestCase):
                         'target image does not match source after mirroring')
 
     def test_cancel(self):
-        self.assert_no_active_block_jobs()
-
         result = self.vm.qmp('drive-mirror', job_id='job0', device='quorum0',
                              sync='full', node_name="repair0", replaces="img1",
                              target=quorum_repair_img, format=iotests.imgfmt)
@@ -932,8 +928,6 @@ class TestRepairQuorum(iotests.QMPTestCase):
         self.assert_has_block_node(None, quorum_img3)
 
     def test_cancel_after_ready(self):
-        self.assert_no_active_block_jobs()
-
         result = self.vm.qmp('drive-mirror', job_id='job0', device='quorum0',
                              sync='full', node_name="repair0", replaces="img1",
                              target=quorum_repair_img, format=iotests.imgfmt)
@@ -948,8 +942,6 @@ class TestRepairQuorum(iotests.QMPTestCase):
                         'target image does not match source after mirroring')
 
     def test_pause(self):
-        self.assert_no_active_block_jobs()
-
         result = self.vm.qmp('drive-mirror', job_id='job0', device='quorum0',
                              sync='full', node_name="repair0", replaces="img1",
                              target=quorum_repair_img, format=iotests.imgfmt)
-- 
2.25.3

[PULL 05/30] iotests/055: refactor compressed backup to vmdk

[Kevin Wolf]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Instead of looping in each test, let's better refactor vmdk target case
as a subclass.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200430124713.3067-6-vsementsov@virtuozzo.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/055     | 70 ++++++++++++++++++++------------------
 tests/qemu-iotests/055.out |  4 +--
 2 files changed, 39 insertions(+), 35 deletions(-)

diff --git a/tests/qemu-iotests/055 b/tests/qemu-iotests/055
index 4175fff5e4..d9e8985167 100755
--- a/tests/qemu-iotests/055
+++ b/tests/qemu-iotests/055
@@ -450,10 +450,9 @@ class TestSingleTransaction(iotests.QMPTestCase):
         self.assert_no_active_block_jobs()
 
 
-class TestDriveCompression(iotests.QMPTestCase):
+class TestCompressedToQcow2(iotests.QMPTestCase):
     image_len = 64 * 1024 * 1024 # MB
-    fmt_supports_compression = [{'type': 'qcow2', 'args': ()},
-                                {'type': 'vmdk', 'args': ('-o', 'subformat=streamOptimized')}]
+    target_fmt = {'type': 'qcow2', 'args': ()}
 
     def tearDown(self):
         self.vm.shutdown()
@@ -463,19 +462,20 @@ class TestDriveCompression(iotests.QMPTestCase):
         except OSError:
             pass
 
-    def do_prepare_drives(self, fmt, args, attach_target):
+    def do_prepare_drives(self, attach_target):
         self.vm = iotests.VM().add_drive('blkdebug::' + test_img)
 
-        qemu_img('create', '-f', fmt, blockdev_target_img,
-                 str(TestDriveCompression.image_len), *args)
+        qemu_img('create', '-f', self.target_fmt['type'], blockdev_target_img,
+                 str(self.image_len), *self.target_fmt['args'])
         if attach_target:
             self.vm.add_drive(blockdev_target_img,
-                              img_format=fmt, interface="none")
+                              img_format=self.target_fmt['type'],
+                              interface="none")
 
         self.vm.launch()
 
-    def do_test_compress_complete(self, cmd, format, attach_target, **args):
-        self.do_prepare_drives(format['type'], format['args'], attach_target)
+    def do_test_compress_complete(self, cmd, attach_target, **args):
+        self.do_prepare_drives(attach_target)
 
         self.assert_no_active_block_jobs()
 
@@ -486,21 +486,21 @@ class TestDriveCompression(iotests.QMPTestCase):
 
         self.vm.shutdown()
         self.assertTrue(iotests.compare_images(test_img, blockdev_target_img,
-                                               iotests.imgfmt, format['type']),
+                                               iotests.imgfmt,
+                                               self.target_fmt['type']),
                         'target image does not match source after backup')
 
     def test_complete_compress_drive_backup(self):
-        for format in TestDriveCompression.fmt_supports_compression:
-            self.do_test_compress_complete('drive-backup', format, False,
-                                           target=blockdev_target_img, mode='existing')
+        self.do_test_compress_complete('drive-backup', False,
+                                       target=blockdev_target_img,
+                                       mode='existing')
 
     def test_complete_compress_blockdev_backup(self):
-        for format in TestDriveCompression.fmt_supports_compression:
-            self.do_test_compress_complete('blockdev-backup', format, True,
-                                           target='drive1')
+        self.do_test_compress_complete('blockdev-backup',
+                                       True, target='drive1')
 
-    def do_test_compress_cancel(self, cmd, format, attach_target, **args):
-        self.do_prepare_drives(format['type'], format['args'], attach_target)
+    def do_test_compress_cancel(self, cmd, attach_target, **args):
+        self.do_prepare_drives(attach_target)
 
         self.assert_no_active_block_jobs()
 
@@ -514,17 +514,16 @@ class TestDriveCompression(iotests.QMPTestCase):
         self.vm.shutdown()
 
     def test_compress_cancel_drive_backup(self):
-        for format in TestDriveCompression.fmt_supports_compression:
-            self.do_test_compress_cancel('drive-backup', format, False,
-                                         target=blockdev_target_img, mode='existing')
+        self.do_test_compress_cancel('drive-backup', False,
+                                     target=blockdev_target_img,
+                                     mode='existing')
 
     def test_compress_cancel_blockdev_backup(self):
-       for format in TestDriveCompression.fmt_supports_compression:
-            self.do_test_compress_cancel('blockdev-backup', format, True,
-                                         target='drive1')
+        self.do_test_compress_cancel('blockdev-backup', True,
+                                     target='drive1')
 
-    def do_test_compress_pause(self, cmd, format, attach_target, **args):
-        self.do_prepare_drives(format['type'], format['args'], attach_target)
+    def do_test_compress_pause(self, cmd, attach_target, **args):
+        self.do_prepare_drives(attach_target)
 
         self.assert_no_active_block_jobs()
 
@@ -550,18 +549,23 @@ class TestDriveCompression(iotests.QMPTestCase):
 
         self.vm.shutdown()
         self.assertTrue(iotests.compare_images(test_img, blockdev_target_img,
-                                               iotests.imgfmt, format['type']),
+                                               iotests.imgfmt,
+                                               self.target_fmt['type']),
                         'target image does not match source after backup')
 
     def test_compress_pause_drive_backup(self):
-        for format in TestDriveCompression.fmt_supports_compression:
-            self.do_test_compress_pause('drive-backup', format, False,
-                                        target=blockdev_target_img, mode='existing')
+        self.do_test_compress_pause('drive-backup', False,
+                                    target=blockdev_target_img,
+                                    mode='existing')
 
     def test_compress_pause_blockdev_backup(self):
-        for format in TestDriveCompression.fmt_supports_compression:
-            self.do_test_compress_pause('blockdev-backup', format, True,
-                                        target='drive1')
+        self.do_test_compress_pause('blockdev-backup', True,
+                                    target='drive1')
+
+
+class TestCompressedToVmdk(TestCompressedToQcow2):
+    target_fmt = {'type': 'vmdk', 'args': ('-o', 'subformat=streamOptimized')}
+
 
 if __name__ == '__main__':
     iotests.main(supported_fmts=['raw', 'qcow2'],
diff --git a/tests/qemu-iotests/055.out b/tests/qemu-iotests/055.out
index 5ce2f9a2ed..5c26d15c0d 100644
--- a/tests/qemu-iotests/055.out
+++ b/tests/qemu-iotests/055.out
@@ -1,5 +1,5 @@
-..............................
+....................................
 ----------------------------------------------------------------------
-Ran 30 tests
+Ran 36 tests
 
 OK
-- 
2.25.3

[PULL 06/30] iotests/055: skip vmdk target tests if vmdk is not whitelisted

[Kevin Wolf]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200430124713.3067-7-vsementsov@virtuozzo.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/055 | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/qemu-iotests/055 b/tests/qemu-iotests/055
index d9e8985167..e250f798f9 100755
--- a/tests/qemu-iotests/055
+++ b/tests/qemu-iotests/055
@@ -566,6 +566,10 @@ class TestCompressedToQcow2(iotests.QMPTestCase):
 class TestCompressedToVmdk(TestCompressedToQcow2):
     target_fmt = {'type': 'vmdk', 'args': ('-o', 'subformat=streamOptimized')}
 
+    @iotests.skip_if_unsupported(['vmdk'])
+    def setUp(self):
+        pass
+
 
 if __name__ == '__main__':
     iotests.main(supported_fmts=['raw', 'qcow2'],
-- 
2.25.3

[PULL 07/30] iotests/109: mark required formats as required to support whitelisting

[Kevin Wolf]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200430124713.3067-8-vsementsov@virtuozzo.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/109 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/qemu-iotests/109 b/tests/qemu-iotests/109
index a51dd84b3d..5bc2e9b001 100755
--- a/tests/qemu-iotests/109
+++ b/tests/qemu-iotests/109
@@ -42,6 +42,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 _supported_fmt raw
 _supported_proto file
 _supported_os Linux
+_require_drivers qcow qcow2 qed vdi vmdk vpc
 
 qemu_comm_method=qmp
 
-- 
2.25.3

[PULL 08/30] iotests/113: mark bochs as required to support whitelisting

[Kevin Wolf]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200430124713.3067-9-vsementsov@virtuozzo.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/113 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/113 b/tests/qemu-iotests/113
index f2703a2c50..71a65de2e7 100755
--- a/tests/qemu-iotests/113
+++ b/tests/qemu-iotests/113
@@ -37,8 +37,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 . ./common.rc
 . ./common.filter
 
-# Some of these test cases use bochs, but others do use raw, so this
-# is only half a lie.
+# Some of these test cases use bochs, but others do use raw
+_require_drivers bochs
 _supported_fmt raw
 _supported_proto file
 _supported_os Linux
-- 
2.25.3

[PULL 09/30] qcow2: Avoid integer wraparound in qcow2_co_truncate()

[Kevin Wolf]

From: Alberto Garcia <berto@igalia.com>

After commit f01643fb8b47e8a70c04bbf45e0f12a9e5bc54de when an image is
extended and BDRV_REQ_ZERO_WRITE is set then the new clusters are
zeroized.

The code however does not detect correctly situations when the old and
the new end of the image are within the same cluster. The problem can
be reproduced with these steps:

   qemu-img create -f qcow2 backing.qcow2 1M
   qemu-img create -f qcow2 -F qcow2 -b backing.qcow2 top.qcow2
   qemu-img resize --shrink top.qcow2 520k
   qemu-img resize top.qcow2 567k

In the last step offset - zero_start causes an integer wraparound.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Message-Id: <20200504155217.10325-1-berto@igalia.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2.c              | 12 ++++---
 tests/qemu-iotests/292     | 73 ++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/292.out | 24 +++++++++++++
 tests/qemu-iotests/group   |  1 +
 4 files changed, 105 insertions(+), 5 deletions(-)
 create mode 100755 tests/qemu-iotests/292
 create mode 100644 tests/qemu-iotests/292.out

diff --git a/block/qcow2.c b/block/qcow2.c
index ad934109a8..fc7d4b185e 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -4242,15 +4242,17 @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
          * requires a cluster-aligned start. The end may be unaligned if it is
          * at the end of the image (which it is here).
          */
-        ret = qcow2_cluster_zeroize(bs, zero_start, offset - zero_start, 0);
-        if (ret < 0) {
-            error_setg_errno(errp, -ret, "Failed to zero out new clusters");
-            goto fail;
+        if (offset > zero_start) {
+            ret = qcow2_cluster_zeroize(bs, zero_start, offset - zero_start, 0);
+            if (ret < 0) {
+                error_setg_errno(errp, -ret, "Failed to zero out new clusters");
+                goto fail;
+            }
         }
 
         /* Write explicit zeros for the unaligned head */
         if (zero_start > old_length) {
-            uint64_t len = zero_start - old_length;
+            uint64_t len = MIN(zero_start, offset) - old_length;
             uint8_t *buf = qemu_blockalign0(bs, len);
             QEMUIOVector qiov;
             qemu_iovec_init_buf(&qiov, buf, len);
diff --git a/tests/qemu-iotests/292 b/tests/qemu-iotests/292
new file mode 100755
index 0000000000..a2de27cca4
--- /dev/null
+++ b/tests/qemu-iotests/292
@@ -0,0 +1,73 @@
+#!/usr/bin/env bash
+#
+# Test resizing a qcow2 image with a backing file
+#
+# Copyright (C) 2020 Igalia, S.L.
+# Author: Alberto Garcia <berto@igalia.com>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+# creator
+owner=berto@igalia.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+status=1    # failure is the default!
+
+_cleanup()
+{
+    _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt qcow2
+_supported_proto file
+_supported_os Linux
+
+echo '### Create the backing image'
+BACKING_IMG="$TEST_IMG.base"
+TEST_IMG="$BACKING_IMG" _make_test_img 1M
+
+echo '### Fill the backing image with data (0x11)'
+$QEMU_IO -c 'write -P 0x11 0 1M' "$BACKING_IMG" | _filter_qemu_io
+
+echo '### Create the top image'
+_make_test_img -F "$IMGFMT" -b "$BACKING_IMG"
+
+echo '### Fill the top image with data (0x22)'
+$QEMU_IO -c 'write -P 0x22 0 1M' "$TEST_IMG" | _filter_qemu_io
+
+# Both offsets are part of the same cluster.
+echo '### Shrink the image to 520k'
+$QEMU_IMG resize --shrink "$TEST_IMG" 520k
+echo '### Grow the image to 567k'
+$QEMU_IMG resize "$TEST_IMG" 567k
+
+echo '### Check that the tail of the image reads as zeroes'
+$QEMU_IO -c 'read -P 0x22    0 520k' "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c 'read -P 0x00 520k  47k' "$TEST_IMG" | _filter_qemu_io
+
+echo '### Show output of qemu-img map'
+$QEMU_IMG map "$TEST_IMG" | _filter_testdir
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/292.out b/tests/qemu-iotests/292.out
new file mode 100644
index 0000000000..807e0530c3
--- /dev/null
+++ b/tests/qemu-iotests/292.out
@@ -0,0 +1,24 @@
+QA output created by 292
+### Create the backing image
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=1048576
+### Fill the backing image with data (0x11)
+wrote 1048576/1048576 bytes at offset 0
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+### Create the top image
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=IMGFMT
+### Fill the top image with data (0x22)
+wrote 1048576/1048576 bytes at offset 0
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+### Shrink the image to 520k
+Image resized.
+### Grow the image to 567k
+Image resized.
+### Check that the tail of the image reads as zeroes
+read 532480/532480 bytes at offset 0
+520 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 48128/48128 bytes at offset 532480
+47 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+### Show output of qemu-img map
+Offset          Length          Mapped to       File
+0               0x8dc00         0x50000         TEST_DIR/t.qcow2
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 1710470e70..fe649c5b73 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -298,3 +298,4 @@
 288 quick
 289 rw quick
 290 rw auto quick
+292 rw auto quick
-- 
2.25.3

[PULL 10/30] vmdk: Rename VmdkMetaData.valid to new_allocation

[Kevin Wolf]

m_data is used for zero clusters even though valid == 0. It really only
means that a new cluster was allocated in the image file. Rename it to
reflect this.

While at it, change it from int to bool, too.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20200430133007.170335-2-kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/vmdk.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index b02fdd14b2..ecfb4a86d2 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -180,7 +180,7 @@ typedef struct VmdkMetaData {
     unsigned int l1_index;
     unsigned int l2_index;
     unsigned int l2_offset;
-    int valid;
+    bool new_allocation;
     uint32_t *l2_cache_entry;
 } VmdkMetaData;
 
@@ -1492,7 +1492,7 @@ static int get_cluster_offset(BlockDriverState *bs,
     unsigned int l2_size_bytes = extent->l2_size * extent->entry_size;
 
     if (m_data) {
-        m_data->valid = 0;
+        m_data->new_allocation = false;
     }
     if (extent->flat) {
         *cluster_offset = extent->flat_start_offset;
@@ -1630,7 +1630,7 @@ static int get_cluster_offset(BlockDriverState *bs,
             return ret;
         }
         if (m_data) {
-            m_data->valid = 1;
+            m_data->new_allocation = true;
             m_data->l1_index = l1_index;
             m_data->l2_index = l2_index;
             m_data->l2_offset = l2_offset;
@@ -2021,7 +2021,7 @@ static int vmdk_pwritev(BlockDriverState *bs, uint64_t offset,
             if (ret) {
                 return ret;
             }
-            if (m_data.valid) {
+            if (m_data.new_allocation) {
                 /* update L2 tables */
                 if (vmdk_L2update(extent, &m_data,
                                   cluster_offset >> BDRV_SECTOR_BITS)
-- 
2.25.3

[PULL 11/30] vmdk: Fix zero cluster allocation

[Kevin Wolf]

m_data must contain valid data even for zero clusters when no cluster
was allocated in the image file. Without this, zero writes segfault with
images that have zeroed_grain=on.

For zero writes, we don't want to allocate a cluster in the image file
even in compressed files.

Fixes: 524089bce43fd1cd3daaca979872451efa2cf7c6
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20200430133007.170335-3-kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/vmdk.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index ecfb4a86d2..fcd6b38d64 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1572,6 +1572,12 @@ static int get_cluster_offset(BlockDriverState *bs,
     extent->l2_cache_counts[min_index] = 1;
  found:
     l2_index = ((offset >> 9) / extent->cluster_sectors) % extent->l2_size;
+    if (m_data) {
+        m_data->l1_index = l1_index;
+        m_data->l2_index = l2_index;
+        m_data->l2_offset = l2_offset;
+        m_data->l2_cache_entry = ((uint32_t *)l2_table) + l2_index;
+    }
 
     if (extent->sesparse) {
         cluster_sector = le64_to_cpu(((uint64_t *)l2_table)[l2_index]);
@@ -1631,10 +1637,6 @@ static int get_cluster_offset(BlockDriverState *bs,
         }
         if (m_data) {
             m_data->new_allocation = true;
-            m_data->l1_index = l1_index;
-            m_data->l2_index = l2_index;
-            m_data->l2_offset = l2_offset;
-            m_data->l2_cache_entry = ((uint32_t *)l2_table) + l2_index;
         }
     }
     *cluster_offset = cluster_sector << BDRV_SECTOR_BITS;
@@ -1990,7 +1992,7 @@ static int vmdk_pwritev(BlockDriverState *bs, uint64_t offset,
                 error_report("Could not write to allocated cluster"
                               " for streamOptimized");
                 return -EIO;
-            } else {
+            } else if (!zeroed) {
                 /* allocate */
                 ret = get_cluster_offset(bs, extent, &m_data, offset,
                                          true, &cluster_offset, 0, 0);
-- 
2.25.3

[PULL 12/30] vmdk: Fix partial overwrite of zero cluster

[Kevin Wolf]

When overwriting a zero cluster, we must not perform copy-on-write from
the backing file, but from a zeroed buffer.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20200430133007.170335-4-kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/vmdk.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index fcd6b38d64..ab8eec1f27 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1340,7 +1340,9 @@ static void vmdk_refresh_limits(BlockDriverState *bs, Error **errp)
  * get_whole_cluster
  *
  * Copy backing file's cluster that covers @sector_num, otherwise write zero,
- * to the cluster at @cluster_sector_num.
+ * to the cluster at @cluster_sector_num. If @zeroed is true, we're overwriting
+ * a zeroed cluster in the current layer and must not copy data from the
+ * backing file.
  *
  * If @skip_start_sector < @skip_end_sector, the relative range
  * [@skip_start_sector, @skip_end_sector) is not copied or written, and leave
@@ -1351,18 +1353,21 @@ static int get_whole_cluster(BlockDriverState *bs,
                              uint64_t cluster_offset,
                              uint64_t offset,
                              uint64_t skip_start_bytes,
-                             uint64_t skip_end_bytes)
+                             uint64_t skip_end_bytes,
+                             bool zeroed)
 {
     int ret = VMDK_OK;
     int64_t cluster_bytes;
     uint8_t *whole_grain;
+    bool copy_from_backing;
 
     /* For COW, align request sector_num to cluster start */
     cluster_bytes = extent->cluster_sectors << BDRV_SECTOR_BITS;
     offset = QEMU_ALIGN_DOWN(offset, cluster_bytes);
     whole_grain = qemu_blockalign(bs, cluster_bytes);
+    copy_from_backing = bs->backing && !zeroed;
 
-    if (!bs->backing) {
+    if (!copy_from_backing) {
         memset(whole_grain, 0, skip_start_bytes);
         memset(whole_grain + skip_end_bytes, 0, cluster_bytes - skip_end_bytes);
     }
@@ -1377,7 +1382,7 @@ static int get_whole_cluster(BlockDriverState *bs,
 
     /* Read backing data before skip range */
     if (skip_start_bytes > 0) {
-        if (bs->backing) {
+        if (copy_from_backing) {
             /* qcow2 emits this on bs->file instead of bs->backing */
             BLKDBG_EVENT(extent->file, BLKDBG_COW_READ);
             ret = bdrv_pread(bs->backing, offset, whole_grain,
@@ -1397,7 +1402,7 @@ static int get_whole_cluster(BlockDriverState *bs,
     }
     /* Read backing data after skip range */
     if (skip_end_bytes < cluster_bytes) {
-        if (bs->backing) {
+        if (copy_from_backing) {
             /* qcow2 emits this on bs->file instead of bs->backing */
             BLKDBG_EVENT(extent->file, BLKDBG_COW_READ);
             ret = bdrv_pread(bs->backing, offset + skip_end_bytes,
@@ -1631,7 +1636,8 @@ static int get_cluster_offset(BlockDriverState *bs,
          * or inappropriate VM shutdown.
          */
         ret = get_whole_cluster(bs, extent, cluster_sector * BDRV_SECTOR_SIZE,
-                                offset, skip_start_bytes, skip_end_bytes);
+                                offset, skip_start_bytes, skip_end_bytes,
+                                zeroed);
         if (ret) {
             return ret;
         }
-- 
2.25.3

[PULL 13/30] vmdk: Don't update L2 table for zero write on zero cluster

[Kevin Wolf]

If a cluster is already zeroed, we don't have to call vmdk_L2update(),
which is rather slow because it flushes the image file.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20200430133007.170335-5-kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/vmdk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index ab8eec1f27..2880d88dea 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -2013,7 +2013,7 @@ static int vmdk_pwritev(BlockDriverState *bs, uint64_t offset,
                     offset_in_cluster == 0 &&
                     n_bytes >= extent->cluster_sectors * BDRV_SECTOR_SIZE) {
                 n_bytes = extent->cluster_sectors * BDRV_SECTOR_SIZE;
-                if (!zero_dry_run) {
+                if (!zero_dry_run && ret != VMDK_ZEROED) {
                     /* update L2 tables */
                     if (vmdk_L2update(extent, &m_data, VMDK_GTE_ZEROED)
                             != VMDK_OK) {
-- 
2.25.3

[PULL 14/30] vmdk: Flush only once in vmdk_L2update()

[Kevin Wolf]

If we have a backup L2 table, we currently flush once after writing to
the active L2 table and again after writing to the backup table. A
single flush is enough and makes things a little less slow.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20200430133007.170335-6-kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/vmdk.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 2880d88dea..b18f128816 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1435,7 +1435,7 @@ static int vmdk_L2update(VmdkExtent *extent, VmdkMetaData *m_data,
     offset = cpu_to_le32(offset);
     /* update L2 table */
     BLKDBG_EVENT(extent->file, BLKDBG_L2_UPDATE);
-    if (bdrv_pwrite_sync(extent->file,
+    if (bdrv_pwrite(extent->file,
                 ((int64_t)m_data->l2_offset * 512)
                     + (m_data->l2_index * sizeof(offset)),
                 &offset, sizeof(offset)) < 0) {
@@ -1444,13 +1444,16 @@ static int vmdk_L2update(VmdkExtent *extent, VmdkMetaData *m_data,
     /* update backup L2 table */
     if (extent->l1_backup_table_offset != 0) {
         m_data->l2_offset = extent->l1_backup_table[m_data->l1_index];
-        if (bdrv_pwrite_sync(extent->file,
+        if (bdrv_pwrite(extent->file,
                     ((int64_t)m_data->l2_offset * 512)
                         + (m_data->l2_index * sizeof(offset)),
                     &offset, sizeof(offset)) < 0) {
             return VMDK_ERROR;
         }
     }
+    if (bdrv_flush(extent->file->bs) < 0) {
+        return VMDK_ERROR;
+    }
     if (m_data->l2_cache_entry) {
         *m_data->l2_cache_entry = offset;
     }
-- 
2.25.3

[PULL 15/30] iotests: vmdk: Enable zeroed_grained=on by default

[Kevin Wolf]

In order to avoid bitrot in the zero cluster code in VMDK, enable
zeroed_grain=on by default for the tests.

059 now unsets the default options because zeroed_grain=on works only
with some subformats and the test case tests many different subformats,
including those for which it doesn't work.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20200430133007.170335-7-kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/059   | 6 +++---
 tests/qemu-iotests/check | 3 +++
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059
index 5438025285..4c90fc0363 100755
--- a/tests/qemu-iotests/059
+++ b/tests/qemu-iotests/059
@@ -41,9 +41,9 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 _supported_fmt vmdk
 _supported_proto file
 _supported_os Linux
-_unsupported_imgopts "subformat=monolithicFlat" \
-                     "subformat=twoGbMaxExtentFlat" \
-                     "subformat=twoGbMaxExtentSparse"
+
+# We test all kinds of VMDK options here, so ignore user-specified options
+IMGOPTS=""
 
 capacity_offset=16
 granularity_offset=20
diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index f7a2d3d6c3..9c461cf76d 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -546,6 +546,9 @@ fi
 if [ "$IMGFMT" == "luks" ] && ! (echo "$IMGOPTS" | grep "iter-time=" > /dev/null); then
     IMGOPTS=$(_optstr_add "$IMGOPTS" "iter-time=10")
 fi
+if [ "$IMGFMT" == "vmdk" ] && ! (echo "$IMGOPTS" | grep "zeroed_grain=" > /dev/null); then
+    IMGOPTS=$(_optstr_add "$IMGOPTS" "zeroed_grain=on")
+fi
 
 if [ -z "$SAMPLE_IMG_DIR" ]; then
         SAMPLE_IMG_DIR="$source_iotests/sample_images"
-- 
2.25.3

[PULL 16/30] iotests/283: Use consistent size for source and target

[Kevin Wolf]

The test case forgot to specify the null-co size for the target node.
When adding a check to backup that both sizes match, this would fail
because of the size mismatch and not the behaviour that the test really
wanted to test.

Fixes: a541fcc27c98b96da187c7d4573f3270f3ddd283
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20200430142755.315494-2-kwolf@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/283     | 6 +++++-
 tests/qemu-iotests/283.out | 2 +-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/283 b/tests/qemu-iotests/283
index e17b953333..383797ed62 100644
--- a/tests/qemu-iotests/283
+++ b/tests/qemu-iotests/283
@@ -74,7 +74,11 @@ to check that crash is fixed :)
 vm = iotests.VM()
 vm.launch()
 
-vm.qmp_log('blockdev-add', **{'node-name': 'target', 'driver': 'null-co'})
+vm.qmp_log('blockdev-add', **{
+    'node-name': 'target',
+    'driver': 'null-co',
+    'size': size,
+})
 
 vm.qmp_log('blockdev-add', **{
     'node-name': 'source',
diff --git a/tests/qemu-iotests/283.out b/tests/qemu-iotests/283.out
index daaf5828c1..d8cff22cc1 100644
--- a/tests/qemu-iotests/283.out
+++ b/tests/qemu-iotests/283.out
@@ -1,4 +1,4 @@
-{"execute": "blockdev-add", "arguments": {"driver": "null-co", "node-name": "target"}}
+{"execute": "blockdev-add", "arguments": {"driver": "null-co", "node-name": "target", "size": 1048576}}
 {"return": {}}
 {"execute": "blockdev-add", "arguments": {"driver": "blkdebug", "image": {"driver": "null-co", "node-name": "base", "size": 1048576}, "node-name": "source"}}
 {"return": {}}
-- 
2.25.3

[PULL 17/30] backup: Improve error for bdrv_getlength() failure

[Kevin Wolf]

bdrv_get_device_name() will be an empty string with modern management
tools that don't use -drive. Use bdrv_get_device_or_node_name() instead
so that the node name is used if the BlockBackend is anonymous.

While at it, start with upper case to make the message consistent with
the rest of the function.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Message-Id: <20200430142755.315494-3-kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/backup.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index a7a7dcaf4c..c4c3b8cd46 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -400,8 +400,8 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
 
     len = bdrv_getlength(bs);
     if (len < 0) {
-        error_setg_errno(errp, -len, "unable to get length for '%s'",
-                         bdrv_get_device_name(bs));
+        error_setg_errno(errp, -len, "Unable to get length for '%s'",
+                         bdrv_get_device_or_node_name(bs));
         goto error;
     }
 
-- 
2.25.3

[PULL 18/30] backup: Make sure that source and target size match

[Kevin Wolf]

Since the introduction of a backup filter node in commit 00e30f05d, the
backup block job crashes when the target image is smaller than the
source image because it will try to write after the end of the target
node without having BLK_PERM_RESIZE. (Previously, the BlockBackend layer
would have caught this and errored out gracefully.)

We can fix this and even do better than the old behaviour: Check that
source and target have the same image size at the start of the block job
and unshare BLK_PERM_RESIZE. (This permission was already unshared
before the same commit 00e30f05d, but the BlockBackend that was used to
make the restriction was removed without a replacement.) This will
immediately error out when starting the job instead of only when writing
to a block that doesn't exist in the target.

Longer target than source would technically work because we would never
write to blocks that don't exist, but semantically these are invalid,
too, because a backup is supposed to create a copy, not just an image
that starts with a copy.

Fixes: 00e30f05de1d19586345ec373970ef4c192c6270
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1778593
Cc: qemu-stable@nongnu.org
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20200430142755.315494-4-kwolf@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/backup-top.c | 14 +++++++++-----
 block/backup.c     | 14 +++++++++++++-
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/block/backup-top.c b/block/backup-top.c
index 3b50c06e2c..79b268e6dc 100644
--- a/block/backup-top.c
+++ b/block/backup-top.c
@@ -148,8 +148,10 @@ static void backup_top_child_perm(BlockDriverState *bs, BdrvChild *c,
          *
          * Share write to target (child_file), to not interfere
          * with guest writes to its disk which may be in target backing chain.
+         * Can't resize during a backup block job because we check the size
+         * only upfront.
          */
-        *nshared = BLK_PERM_ALL;
+        *nshared = BLK_PERM_ALL & ~BLK_PERM_RESIZE;
         *nperm = BLK_PERM_WRITE;
     } else {
         /* Source child */
@@ -159,7 +161,7 @@ static void backup_top_child_perm(BlockDriverState *bs, BdrvChild *c,
         if (perm & BLK_PERM_WRITE) {
             *nperm = *nperm | BLK_PERM_CONSISTENT_READ;
         }
-        *nshared &= ~BLK_PERM_WRITE;
+        *nshared &= ~(BLK_PERM_WRITE | BLK_PERM_RESIZE);
     }
 }
 
@@ -192,11 +194,13 @@ BlockDriverState *bdrv_backup_top_append(BlockDriverState *source,
 {
     Error *local_err = NULL;
     BDRVBackupTopState *state;
-    BlockDriverState *top = bdrv_new_open_driver(&bdrv_backup_top_filter,
-                                                 filter_node_name,
-                                                 BDRV_O_RDWR, errp);
+    BlockDriverState *top;
     bool appended = false;
 
+    assert(source->total_sectors == target->total_sectors);
+
+    top = bdrv_new_open_driver(&bdrv_backup_top_filter, filter_node_name,
+                               BDRV_O_RDWR, errp);
     if (!top) {
         return NULL;
     }
diff --git a/block/backup.c b/block/backup.c
index c4c3b8cd46..4f13bb20a5 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -340,7 +340,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
                   BlockCompletionFunc *cb, void *opaque,
                   JobTxn *txn, Error **errp)
 {
-    int64_t len;
+    int64_t len, target_len;
     BackupBlockJob *job = NULL;
     int64_t cluster_size;
     BdrvRequestFlags write_flags;
@@ -405,6 +405,18 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
         goto error;
     }
 
+    target_len = bdrv_getlength(target);
+    if (target_len < 0) {
+        error_setg_errno(errp, -target_len, "Unable to get length for '%s'",
+                         bdrv_get_device_or_node_name(bs));
+        goto error;
+    }
+
+    if (target_len != len) {
+        error_setg(errp, "Source and target image have different sizes");
+        goto error;
+    }
+
     cluster_size = backup_calculate_cluster_size(target, errp);
     if (cluster_size < 0) {
         goto error;
-- 
2.25.3

[PULL 19/30] iotests: Backup with different source/target size

[Kevin Wolf]

This tests that the backup job catches situations where the target node
has a different size than the source node. It must also forbid resize
operations when the job is already running.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20200430142755.315494-5-kwolf@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/055     | 41 ++++++++++++++++++++++++++++++++++++--
 tests/qemu-iotests/055.out |  4 ++--
 2 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/tests/qemu-iotests/055 b/tests/qemu-iotests/055
index e250f798f9..7f8e630de3 100755
--- a/tests/qemu-iotests/055
+++ b/tests/qemu-iotests/055
@@ -48,8 +48,10 @@ class TestSingleDrive(iotests.QMPTestCase):
     def setUp(self):
         qemu_img('create', '-f', iotests.imgfmt, blockdev_target_img, str(image_len))
 
-        self.vm = iotests.VM().add_drive('blkdebug::' + test_img)
-        self.vm.add_drive(blockdev_target_img, interface="none")
+        self.vm = iotests.VM()
+        self.vm.add_drive('blkdebug::' + test_img, 'node-name=source')
+        self.vm.add_drive(blockdev_target_img, 'node-name=target',
+                          interface="none")
         if iotests.qemu_default_machine == 'pc':
             self.vm.add_drive(None, 'media=cdrom', 'ide')
         self.vm.launch()
@@ -112,6 +114,41 @@ class TestSingleDrive(iotests.QMPTestCase):
     def test_pause_blockdev_backup(self):
         self.do_test_pause('blockdev-backup', 'drive1', blockdev_target_img)
 
+    def do_test_resize_blockdev_backup(self, device, node):
+        def pre_finalize():
+            result = self.vm.qmp('block_resize', device=device, size=65536)
+            self.assert_qmp(result, 'error/class', 'GenericError')
+
+            result = self.vm.qmp('block_resize', node_name=node, size=65536)
+            self.assert_qmp(result, 'error/class', 'GenericError')
+
+        result = self.vm.qmp('blockdev-backup', job_id='job0', device='drive0',
+                             target='drive1', sync='full', auto_finalize=False,
+                             auto_dismiss=False)
+        self.assert_qmp(result, 'return', {})
+
+        self.vm.run_job('job0', auto_finalize=False, pre_finalize=pre_finalize)
+
+    def test_source_resize_blockdev_backup(self):
+        self.do_test_resize_blockdev_backup('drive0', 'source')
+
+    def test_target_resize_blockdev_backup(self):
+        self.do_test_resize_blockdev_backup('drive1', 'target')
+
+    def do_test_target_size(self, size):
+        result = self.vm.qmp('block_resize', device='drive1', size=size)
+        self.assert_qmp(result, 'return', {})
+
+        result = self.vm.qmp('blockdev-backup', job_id='job0', device='drive0',
+                             target='drive1', sync='full')
+        self.assert_qmp(result, 'error/class', 'GenericError')
+
+    def test_small_target(self):
+        self.do_test_target_size(image_len // 2)
+
+    def test_large_target(self):
+        self.do_test_target_size(image_len * 2)
+
     def test_medium_not_found(self):
         if iotests.qemu_default_machine != 'pc':
             return
diff --git a/tests/qemu-iotests/055.out b/tests/qemu-iotests/055.out
index 5c26d15c0d..0a5e9583a4 100644
--- a/tests/qemu-iotests/055.out
+++ b/tests/qemu-iotests/055.out
@@ -1,5 +1,5 @@
-....................................
+........................................
 ----------------------------------------------------------------------
-Ran 36 tests
+Ran 40 tests
 
 OK
-- 
2.25.3

[PULL 20/30] iotests/055: Use cache.no-flush for vmdk target

[Kevin Wolf]

055 uses the backup block job to create a compressed backup of an
$IMGFMT image with both qcow2 and vmdk targets. However, cluster
allocation in vmdk is very slow because it flushes the image file after
each L2 update.

There is no reason why we need this level of safety in this test, so
let's disable flushes for vmdk. For the blockdev-backup tests this is
achieved by simply adding the cache.no-flush=on to the drive_add() for
the target. For drive-backup, the caching flags are copied from the
source node, so we'll also add the flag to the source node, even though
it is not vmdk.

This can make the test run significantly faster (though it doesn't make
a difference on tmpfs). In my usual setup it goes from ~45s to ~15s.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20200505064618.16267-1-kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/055 | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/tests/qemu-iotests/055 b/tests/qemu-iotests/055
index 7f8e630de3..4d3744b0d3 100755
--- a/tests/qemu-iotests/055
+++ b/tests/qemu-iotests/055
@@ -489,7 +489,7 @@ class TestSingleTransaction(iotests.QMPTestCase):
 
 class TestCompressedToQcow2(iotests.QMPTestCase):
     image_len = 64 * 1024 * 1024 # MB
-    target_fmt = {'type': 'qcow2', 'args': ()}
+    target_fmt = {'type': 'qcow2', 'args': (), 'drive-opts': ''}
 
     def tearDown(self):
         self.vm.shutdown()
@@ -500,14 +500,16 @@ class TestCompressedToQcow2(iotests.QMPTestCase):
             pass
 
     def do_prepare_drives(self, attach_target):
-        self.vm = iotests.VM().add_drive('blkdebug::' + test_img)
+        self.vm = iotests.VM().add_drive('blkdebug::' + test_img,
+                                         opts=self.target_fmt['drive-opts'])
 
         qemu_img('create', '-f', self.target_fmt['type'], blockdev_target_img,
                  str(self.image_len), *self.target_fmt['args'])
         if attach_target:
             self.vm.add_drive(blockdev_target_img,
                               img_format=self.target_fmt['type'],
-                              interface="none")
+                              interface="none",
+                              opts=self.target_fmt['drive-opts'])
 
         self.vm.launch()
 
@@ -601,7 +603,8 @@ class TestCompressedToQcow2(iotests.QMPTestCase):
 
 
 class TestCompressedToVmdk(TestCompressedToQcow2):
-    target_fmt = {'type': 'vmdk', 'args': ('-o', 'subformat=streamOptimized')}
+    target_fmt = {'type': 'vmdk', 'args': ('-o', 'subformat=streamOptimized'),
+                  'drive-opts': 'cache.no-flush=on'}
 
     @iotests.skip_if_unsupported(['vmdk'])
     def setUp(self):
-- 
2.25.3

[PULL 21/30] qcow2: Fix preallocation on block devices

[Kevin Wolf]

From: Max Reitz <mreitz@redhat.com>

Calling bdrv_getlength() to get the pre-truncate file size will not
really work on block devices, because they have always the same length,
and trying to write beyond it will fail with a rather cryptic error
message.

Instead, we should use qcow2_get_last_cluster() and bdrv_getlength()
only as a fallback.

Before this patch:
$ truncate -s 1G test.img
$ sudo losetup -f --show test.img
/dev/loop0
$ sudo qemu-img create -f qcow2 -o preallocation=full /dev/loop0 64M
Formatting '/dev/loop0', fmt=qcow2 size=67108864 cluster_size=65536
preallocation=full lazy_refcounts=off refcount_bits=16
qemu-img: /dev/loop0: Could not resize image: Failed to resize refcount
structures: No space left on device

With this patch:
$ sudo qemu-img create -f qcow2 -o preallocation=full /dev/loop0 64M
Formatting '/dev/loop0', fmt=qcow2 size=67108864 cluster_size=65536
preallocation=full lazy_refcounts=off refcount_bits=16
qemu-img: /dev/loop0: Could not resize image: Failed to resize
underlying file: Preallocation mode 'full' unsupported for this
non-regular file

So as you can see, it still fails, but now the problem is missing
support on the block device level, so we at least get a better error
message.

Note that we cannot preallocate block devices on truncate by design,
because we do not know what area to preallocate.  Their length is always
the same, the truncate operation does not change it.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200505141801.1096763-1-mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index fc7d4b185e..11903fb547 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -4107,7 +4107,7 @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
     {
         int64_t allocation_start, host_offset, guest_offset;
         int64_t clusters_allocated;
-        int64_t old_file_size, new_file_size;
+        int64_t old_file_size, last_cluster, new_file_size;
         uint64_t nb_new_data_clusters, nb_new_l2_tables;
 
         /* With a data file, preallocation means just allocating the metadata
@@ -4127,7 +4127,13 @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
             ret = old_file_size;
             goto fail;
         }
-        old_file_size = ROUND_UP(old_file_size, s->cluster_size);
+
+        last_cluster = qcow2_get_last_cluster(bs, old_file_size);
+        if (last_cluster >= 0) {
+            old_file_size = (last_cluster + 1) * s->cluster_size;
+        } else {
+            old_file_size = ROUND_UP(old_file_size, s->cluster_size);
+        }
 
         nb_new_data_clusters = DIV_ROUND_UP(offset - old_length,
                                             s->cluster_size);
-- 
2.25.3

[PULL 22/30] gluster: Drop useless has_zero_init callback

[Kevin Wolf]

From: Eric Blake <eblake@redhat.com>

block.c already defaults to 0 if we don't provide a callback; there's
no need to write a callback that always fails.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Message-Id: <20200428202905.770727-2-eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/gluster.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/block/gluster.c b/block/gluster.c
index d06df900f6..31233cac69 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -1359,12 +1359,6 @@ static int64_t qemu_gluster_allocated_file_size(BlockDriverState *bs)
     }
 }
 
-static int qemu_gluster_has_zero_init(BlockDriverState *bs)
-{
-    /* GlusterFS volume could be backed by a block device */
-    return 0;
-}
-
 /*
  * Find allocation range in @bs around offset @start.
  * May change underlying file descriptor's file offset.
@@ -1569,8 +1563,6 @@ static BlockDriver bdrv_gluster = {
     .bdrv_co_readv                = qemu_gluster_co_readv,
     .bdrv_co_writev               = qemu_gluster_co_writev,
     .bdrv_co_flush_to_disk        = qemu_gluster_co_flush_to_disk,
-    .bdrv_has_zero_init           = qemu_gluster_has_zero_init,
-    .bdrv_has_zero_init_truncate  = qemu_gluster_has_zero_init,
 #ifdef CONFIG_GLUSTERFS_DISCARD
     .bdrv_co_pdiscard             = qemu_gluster_co_pdiscard,
 #endif
@@ -1601,8 +1593,6 @@ static BlockDriver bdrv_gluster_tcp = {
     .bdrv_co_readv                = qemu_gluster_co_readv,
     .bdrv_co_writev               = qemu_gluster_co_writev,
     .bdrv_co_flush_to_disk        = qemu_gluster_co_flush_to_disk,
-    .bdrv_has_zero_init           = qemu_gluster_has_zero_init,
-    .bdrv_has_zero_init_truncate  = qemu_gluster_has_zero_init,
 #ifdef CONFIG_GLUSTERFS_DISCARD
     .bdrv_co_pdiscard             = qemu_gluster_co_pdiscard,
 #endif
@@ -1633,8 +1623,6 @@ static BlockDriver bdrv_gluster_unix = {
     .bdrv_co_readv                = qemu_gluster_co_readv,
     .bdrv_co_writev               = qemu_gluster_co_writev,
     .bdrv_co_flush_to_disk        = qemu_gluster_co_flush_to_disk,
-    .bdrv_has_zero_init           = qemu_gluster_has_zero_init,
-    .bdrv_has_zero_init_truncate  = qemu_gluster_has_zero_init,
 #ifdef CONFIG_GLUSTERFS_DISCARD
     .bdrv_co_pdiscard             = qemu_gluster_co_pdiscard,
 #endif
@@ -1671,8 +1659,6 @@ static BlockDriver bdrv_gluster_rdma = {
     .bdrv_co_readv                = qemu_gluster_co_readv,
     .bdrv_co_writev               = qemu_gluster_co_writev,
     .bdrv_co_flush_to_disk        = qemu_gluster_co_flush_to_disk,
-    .bdrv_has_zero_init           = qemu_gluster_has_zero_init,
-    .bdrv_has_zero_init_truncate  = qemu_gluster_has_zero_init,
 #ifdef CONFIG_GLUSTERFS_DISCARD
     .bdrv_co_pdiscard             = qemu_gluster_co_pdiscard,
 #endif
-- 
2.25.3

[PULL 23/30] file-win32: Support BDRV_REQ_ZERO_WRITE for truncate

[Kevin Wolf]

From: Eric Blake <eblake@redhat.com>

When using bdrv_file, .bdrv_has_zero_init_truncate always returns 1;
therefore, we can behave just like file-posix, and always implement
BDRV_REQ_ZERO_WRITE by ignoring it since the OS gives it to us for
free (note that file-posix.c had to use an 'if' because it shared code
between regular files and block devices, but in file-win32.c,
bdrv_host_device uses a separate .bdrv_file_open).

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200428202905.770727-3-eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/file-win32.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/block/file-win32.c b/block/file-win32.c
index a6b0dda5c3..fa569685d8 100644
--- a/block/file-win32.c
+++ b/block/file-win32.c
@@ -408,6 +408,9 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags,
         win32_aio_attach_aio_context(s->aio, bdrv_get_aio_context(bs));
     }
 
+    /* When extending regular files, we get zeros from the OS */
+    bs->supported_truncate_flags = BDRV_REQ_ZERO_WRITE;
+
     ret = 0;
 fail:
     qemu_opts_del(opts);
-- 
2.25.3

[PULL 24/30] nfs: Support BDRV_REQ_ZERO_WRITE for truncate

[Kevin Wolf]

From: Eric Blake <eblake@redhat.com>

Our .bdrv_has_zero_init_truncate returns 1 if we detect that the OS
always 0-fills; we can use that same knowledge to implement
BDRV_REQ_ZERO_WRITE by ignoring it when the OS gives it to us for
free.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200428202905.770727-4-eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/nfs.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/block/nfs.c b/block/nfs.c
index 385d756e1d..76daa7c9f6 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -620,6 +620,9 @@ static int nfs_file_open(BlockDriverState *bs, QDict *options, int flags,
     }
 
     bs->total_sectors = ret;
+    if (client->has_zero_init) {
+        bs->supported_truncate_flags = BDRV_REQ_ZERO_WRITE;
+    }
     return 0;
 }
 
-- 
2.25.3

[PULL 25/30] rbd: Support BDRV_REQ_ZERO_WRITE for truncate

[Kevin Wolf]

From: Eric Blake <eblake@redhat.com>

Our .bdrv_has_zero_init_truncate always returns 1 because rbd always
0-fills; we can use that same knowledge to implement
BDRV_REQ_ZERO_WRITE by ignoring it.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200428202905.770727-5-eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/rbd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/block/rbd.c b/block/rbd.c
index f2d52091c7..331c45adb2 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -817,6 +817,9 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,
         }
     }
 
+    /* When extending regular files, we get zeros from the OS */
+    bs->supported_truncate_flags = BDRV_REQ_ZERO_WRITE;
+
     r = 0;
     goto out;
 
-- 
2.25.3

[PULL 26/30] sheepdog: Support BDRV_REQ_ZERO_WRITE for truncate

[Kevin Wolf]

From: Eric Blake <eblake@redhat.com>

Our .bdrv_has_zero_init_truncate always returns 1 because sheepdog
always 0-fills; we can use that same knowledge to implement
BDRV_REQ_ZERO_WRITE by ignoring it.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200428202905.770727-6-eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/sheepdog.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index 2eb61938ff..739e6dee30 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1654,6 +1654,7 @@ static int sd_open(BlockDriverState *bs, QDict *options, int flags,
     memcpy(&s->inode, buf, sizeof(s->inode));
 
     bs->total_sectors = s->inode.vdi_size / BDRV_SECTOR_SIZE;
+    bs->supported_truncate_flags = BDRV_REQ_ZERO_WRITE;
     pstrcpy(s->name, sizeof(s->name), vdi);
     qemu_co_mutex_init(&s->lock);
     qemu_co_mutex_init(&s->queue_lock);
-- 
2.25.3

[PULL 27/30] ssh: Support BDRV_REQ_ZERO_WRITE for truncate

[Kevin Wolf]

From: Eric Blake <eblake@redhat.com>

Our .bdrv_has_zero_init_truncate can detect when the remote side
always zero fills; we can reuse that same knowledge to implement
BDRV_REQ_ZERO_WRITE by ignoring it when the server gives it to us for
free.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200428202905.770727-7-eblake@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/ssh.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/block/ssh.c b/block/ssh.c
index 9eb33df859..f9e08a4900 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -883,6 +883,10 @@ static int ssh_file_open(BlockDriverState *bs, QDict *options, int bdrv_flags,
     /* Go non-blocking. */
     ssh_set_blocking(s->session, 0);
 
+    if (s->attrs->type == SSH_FILEXFER_TYPE_REGULAR) {
+        bs->supported_truncate_flags = BDRV_REQ_ZERO_WRITE;
+    }
+
     qapi_free_BlockdevOptionsSsh(opts);
 
     return 0;
-- 
2.25.3

[PULL 28/30] parallels: Rework truncation logic

[Kevin Wolf]

From: Eric Blake <eblake@redhat.com>

The parallels driver tries to use truncation for image growth, but can
only do so when reads are guaranteed as zero.  Now that we have a way
to request zero contents from truncation, we can defer the decision to
actual allocation attempts rather than up front, reducing the number
of places that still use bdrv_has_zero_init_truncate.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200428202905.770727-8-eblake@redhat.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/parallels.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/block/parallels.c b/block/parallels.c
index 8db64a55e3..e7717c508e 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -166,7 +166,7 @@ static int64_t block_status(BDRVParallelsState *s, int64_t sector_num,
 static int64_t allocate_clusters(BlockDriverState *bs, int64_t sector_num,
                                  int nb_sectors, int *pnum)
 {
-    int ret;
+    int ret = 0;
     BDRVParallelsState *s = bs->opaque;
     int64_t pos, space, idx, to_allocate, i, len;
 
@@ -196,14 +196,24 @@ static int64_t allocate_clusters(BlockDriverState *bs, int64_t sector_num,
     }
     if (s->data_end + space > (len >> BDRV_SECTOR_BITS)) {
         space += s->prealloc_size;
+        /*
+         * We require the expanded size to read back as zero. If the
+         * user permitted truncation, we try that; but if it fails, we
+         * force the safer-but-slower fallocate.
+         */
+        if (s->prealloc_mode == PRL_PREALLOC_MODE_TRUNCATE) {
+            ret = bdrv_truncate(bs->file,
+                                (s->data_end + space) << BDRV_SECTOR_BITS,
+                                false, PREALLOC_MODE_OFF, BDRV_REQ_ZERO_WRITE,
+                                NULL);
+            if (ret == -ENOTSUP) {
+                s->prealloc_mode = PRL_PREALLOC_MODE_FALLOCATE;
+            }
+        }
         if (s->prealloc_mode == PRL_PREALLOC_MODE_FALLOCATE) {
             ret = bdrv_pwrite_zeroes(bs->file,
                                      s->data_end << BDRV_SECTOR_BITS,
                                      space << BDRV_SECTOR_BITS, 0);
-        } else {
-            ret = bdrv_truncate(bs->file,
-                                (s->data_end + space) << BDRV_SECTOR_BITS,
-                                false, PREALLOC_MODE_OFF, 0, NULL);
         }
         if (ret < 0) {
             return ret;
@@ -828,6 +838,7 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
         qemu_opt_get_size_del(opts, PARALLELS_OPT_PREALLOC_SIZE, 0);
     s->prealloc_size = MAX(s->tracks, s->prealloc_size >> BDRV_SECTOR_BITS);
     buf = qemu_opt_get_del(opts, PARALLELS_OPT_PREALLOC_MODE);
+    /* prealloc_mode can be downgraded later during allocate_clusters */
     s->prealloc_mode = qapi_enum_parse(&prealloc_mode_lookup, buf,
                                        PRL_PREALLOC_MODE_FALLOCATE,
                                        &local_err);
@@ -836,10 +847,6 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
         goto fail_options;
     }
 
-    if (!bdrv_has_zero_init_truncate(bs->file->bs)) {
-        s->prealloc_mode = PRL_PREALLOC_MODE_FALLOCATE;
-    }
-
     if ((flags & BDRV_O_RDWR) && !(flags & BDRV_O_INACTIVE)) {
         s->header->inuse = cpu_to_le32(HEADER_INUSE_MAGIC);
         ret = parallels_update_header(bs);
-- 
2.25.3

[PULL 29/30] vhdx: Rework truncation logic

[Kevin Wolf]

From: Eric Blake <eblake@redhat.com>

The vhdx driver uses truncation for image growth, with a special case
for blocks that already read as zero but which are only being
partially written.  But with a bit of rearranging, it's just as easy
to defer the decision on whether truncation resulted in zeroes to the
actual allocation attempt, reducing the number of places that still
use bdrv_has_zero_init_truncate.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200428202905.770727-9-eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/vhdx.c | 89 ++++++++++++++++++++++++++++++----------------------
 1 file changed, 51 insertions(+), 38 deletions(-)

diff --git a/block/vhdx.c b/block/vhdx.c
index e11fb7413a..53e756438a 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1240,12 +1240,16 @@ exit:
 /*
  * Allocate a new payload block at the end of the file.
  *
- * Allocation will happen at 1MB alignment inside the file
+ * Allocation will happen at 1MB alignment inside the file.
+ *
+ * If @need_zero is set on entry but not cleared on return, then truncation
+ * could not guarantee that the new portion reads as zero, and the caller
+ * will take care of it instead.
  *
  * Returns the file offset start of the new payload block
  */
 static int vhdx_allocate_block(BlockDriverState *bs, BDRVVHDXState *s,
-                                    uint64_t *new_offset)
+                               uint64_t *new_offset, bool *need_zero)
 {
     int64_t current_len;
 
@@ -1262,6 +1266,17 @@ static int vhdx_allocate_block(BlockDriverState *bs, BDRVVHDXState *s,
         return -EINVAL;
     }
 
+    if (*need_zero) {
+        int ret;
+
+        ret = bdrv_truncate(bs->file, *new_offset + s->block_size, false,
+                            PREALLOC_MODE_OFF, BDRV_REQ_ZERO_WRITE, NULL);
+        if (ret != -ENOTSUP) {
+            *need_zero = false;
+            return ret;
+        }
+    }
+
     return bdrv_truncate(bs->file, *new_offset + s->block_size, false,
                          PREALLOC_MODE_OFF, 0, NULL);
 }
@@ -1355,18 +1370,38 @@ static coroutine_fn int vhdx_co_writev(BlockDriverState *bs, int64_t sector_num,
                 /* in this case, we need to preserve zero writes for
                  * data that is not part of this write, so we must pad
                  * the rest of the buffer to zeroes */
-
-                /* if we are on a posix system with ftruncate() that extends
-                 * a file, then it is zero-filled for us.  On Win32, the raw
-                 * layer uses SetFilePointer and SetFileEnd, which does not
-                 * zero fill AFAIK */
-
-                /* Queue another write of zero buffers if the underlying file
-                 * does not zero-fill on file extension */
-
-                if (bdrv_has_zero_init_truncate(bs->file->bs) == 0) {
-                    use_zero_buffers = true;
-
+                use_zero_buffers = true;
+                /* fall through */
+            case PAYLOAD_BLOCK_NOT_PRESENT: /* fall through */
+            case PAYLOAD_BLOCK_UNMAPPED:
+            case PAYLOAD_BLOCK_UNMAPPED_v095:
+            case PAYLOAD_BLOCK_UNDEFINED:
+                bat_prior_offset = sinfo.file_offset;
+                ret = vhdx_allocate_block(bs, s, &sinfo.file_offset,
+                                          &use_zero_buffers);
+                if (ret < 0) {
+                    goto exit;
+                }
+                /*
+                 * once we support differencing files, this may also be
+                 * partially present
+                 */
+                /* update block state to the newly specified state */
+                vhdx_update_bat_table_entry(bs, s, &sinfo, &bat_entry,
+                                            &bat_entry_offset,
+                                            PAYLOAD_BLOCK_FULLY_PRESENT);
+                bat_update = true;
+                /*
+                 * Since we just allocated a block, file_offset is the
+                 * beginning of the payload block. It needs to be the
+                 * write address, which includes the offset into the
+                 * block, unless the entire block needs to read as
+                 * zeroes but truncation was not able to provide them,
+                 * in which case we need to fill in the rest.
+                 */
+                if (!use_zero_buffers) {
+                    sinfo.file_offset += sinfo.block_offset;
+                } else {
                     /* zero fill the front, if any */
                     if (sinfo.block_offset) {
                         iov1.iov_len = sinfo.block_offset;
@@ -1378,7 +1413,7 @@ static coroutine_fn int vhdx_co_writev(BlockDriverState *bs, int64_t sector_num,
                     }
 
                     /* our actual data */
-                    qemu_iovec_concat(&hd_qiov, qiov,  bytes_done,
+                    qemu_iovec_concat(&hd_qiov, qiov, bytes_done,
                                       sinfo.bytes_avail);
 
                     /* zero fill the back, if any */
@@ -1393,29 +1428,7 @@ static coroutine_fn int vhdx_co_writev(BlockDriverState *bs, int64_t sector_num,
                         sectors_to_write += iov2.iov_len >> BDRV_SECTOR_BITS;
                     }
                 }
-                /* fall through */
-            case PAYLOAD_BLOCK_NOT_PRESENT: /* fall through */
-            case PAYLOAD_BLOCK_UNMAPPED:
-            case PAYLOAD_BLOCK_UNMAPPED_v095:
-            case PAYLOAD_BLOCK_UNDEFINED:
-                bat_prior_offset = sinfo.file_offset;
-                ret = vhdx_allocate_block(bs, s, &sinfo.file_offset);
-                if (ret < 0) {
-                    goto exit;
-                }
-                /* once we support differencing files, this may also be
-                 * partially present */
-                /* update block state to the newly specified state */
-                vhdx_update_bat_table_entry(bs, s, &sinfo, &bat_entry,
-                                            &bat_entry_offset,
-                                            PAYLOAD_BLOCK_FULLY_PRESENT);
-                bat_update = true;
-                /* since we just allocated a block, file_offset is the
-                 * beginning of the payload block. It needs to be the
-                 * write address, which includes the offset into the block */
-                if (!use_zero_buffers) {
-                    sinfo.file_offset += sinfo.block_offset;
-                }
+
                 /* fall through */
             case PAYLOAD_BLOCK_FULLY_PRESENT:
                 /* if the file offset address is in the header zone,
-- 
2.25.3

[PULL 30/30] block: Drop unused .bdrv_has_zero_init_truncate

[Kevin Wolf]

From: Eric Blake <eblake@redhat.com>

Now that there are no clients of bdrv_has_zero_init_truncate, none of
the drivers need to worry about providing it.

What's more, this eliminates a source of some confusion: a literal
reading of the documentation as written in ceaca56f and implemented in
commit 1dcaf527 claims that a driver which returns 0 for
bdrv_has_zero_init_truncate() must not return 1 for
bdrv_has_zero_init(); this condition was violated for parallels, qcow,
and sometimes for vdi, although in practice it did not matter since
those drivers also lacked .bdrv_co_truncate.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200428202905.770727-10-eblake@redhat.com>
Acked-by: Richard W.M. Jones <rjones@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 include/block/block.h     |  1 -
 include/block/block_int.h |  7 -------
 block.c                   | 21 ---------------------
 block/file-posix.c        |  1 -
 block/file-win32.c        |  1 -
 block/nfs.c               |  1 -
 block/qcow2.c             |  1 -
 block/qed.c               |  1 -
 block/raw-format.c        |  6 ------
 block/rbd.c               |  1 -
 block/sheepdog.c          |  3 ---
 block/ssh.c               |  1 -
 12 files changed, 45 deletions(-)

diff --git a/include/block/block.h b/include/block/block.h
index 8b62429aa4..4de8d8f8a6 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -430,7 +430,6 @@ int bdrv_pdiscard(BdrvChild *child, int64_t offset, int64_t bytes);
 int bdrv_co_pdiscard(BdrvChild *child, int64_t offset, int64_t bytes);
 int bdrv_has_zero_init_1(BlockDriverState *bs);
 int bdrv_has_zero_init(BlockDriverState *bs);
-int bdrv_has_zero_init_truncate(BlockDriverState *bs);
 bool bdrv_unallocated_blocks_are_zero(BlockDriverState *bs);
 bool bdrv_can_write_zeroes_with_unmap(BlockDriverState *bs);
 int bdrv_block_status(BlockDriverState *bs, int64_t offset,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 92335f33c7..df6d0273d6 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -449,16 +449,9 @@ struct BlockDriver {
     /*
      * Returns 1 if newly created images are guaranteed to contain only
      * zeros, 0 otherwise.
-     * Must return 0 if .bdrv_has_zero_init_truncate() returns 0.
      */
     int (*bdrv_has_zero_init)(BlockDriverState *bs);
 
-    /*
-     * Returns 1 if new areas added by growing the image with
-     * PREALLOC_MODE_OFF contain only zeros, 0 otherwise.
-     */
-    int (*bdrv_has_zero_init_truncate)(BlockDriverState *bs);
-
     /* Remove fd handlers, timers, and other event loop callbacks so the event
      * loop is no longer in use.  Called with no in-flight requests and in
      * depth-first traversal order with parents before child nodes.
diff --git a/block.c b/block.c
index cf5c19b1db..0653ccb913 100644
--- a/block.c
+++ b/block.c
@@ -5284,27 +5284,6 @@ int bdrv_has_zero_init(BlockDriverState *bs)
     return 0;
 }
 
-int bdrv_has_zero_init_truncate(BlockDriverState *bs)
-{
-    if (!bs->drv) {
-        return 0;
-    }
-
-    if (bs->backing) {
-        /* Depends on the backing image length, but better safe than sorry */
-        return 0;
-    }
-    if (bs->drv->bdrv_has_zero_init_truncate) {
-        return bs->drv->bdrv_has_zero_init_truncate(bs);
-    }
-    if (bs->file && bs->drv->is_filter) {
-        return bdrv_has_zero_init_truncate(bs->file->bs);
-    }
-
-    /* safe default */
-    return 0;
-}
-
 bool bdrv_unallocated_blocks_are_zero(BlockDriverState *bs)
 {
     BlockDriverInfo bdi;
diff --git a/block/file-posix.c b/block/file-posix.c
index 05e094be29..3ab8f5a0fa 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -3100,7 +3100,6 @@ BlockDriver bdrv_file = {
     .bdrv_co_create = raw_co_create,
     .bdrv_co_create_opts = raw_co_create_opts,
     .bdrv_has_zero_init = bdrv_has_zero_init_1,
-    .bdrv_has_zero_init_truncate = bdrv_has_zero_init_1,
     .bdrv_co_block_status = raw_co_block_status,
     .bdrv_co_invalidate_cache = raw_co_invalidate_cache,
     .bdrv_co_pwrite_zeroes = raw_co_pwrite_zeroes,
diff --git a/block/file-win32.c b/block/file-win32.c
index fa569685d8..221aaf713e 100644
--- a/block/file-win32.c
+++ b/block/file-win32.c
@@ -641,7 +641,6 @@ BlockDriver bdrv_file = {
     .bdrv_close         = raw_close,
     .bdrv_co_create_opts = raw_co_create_opts,
     .bdrv_has_zero_init = bdrv_has_zero_init_1,
-    .bdrv_has_zero_init_truncate = bdrv_has_zero_init_1,
 
     .bdrv_aio_preadv    = raw_aio_preadv,
     .bdrv_aio_pwritev   = raw_aio_pwritev,
diff --git a/block/nfs.c b/block/nfs.c
index 76daa7c9f6..b1718d125a 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -872,7 +872,6 @@ static BlockDriver bdrv_nfs = {
     .create_opts                    = &nfs_create_opts,
 
     .bdrv_has_zero_init             = nfs_has_zero_init,
-    .bdrv_has_zero_init_truncate    = nfs_has_zero_init,
     .bdrv_get_allocated_file_size   = nfs_get_allocated_file_size,
     .bdrv_co_truncate               = nfs_file_co_truncate,
 
diff --git a/block/qcow2.c b/block/qcow2.c
index 11903fb547..1ad95ff048 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -5621,7 +5621,6 @@ BlockDriver bdrv_qcow2 = {
     .bdrv_co_create_opts  = qcow2_co_create_opts,
     .bdrv_co_create       = qcow2_co_create,
     .bdrv_has_zero_init   = qcow2_has_zero_init,
-    .bdrv_has_zero_init_truncate = bdrv_has_zero_init_1,
     .bdrv_co_block_status = qcow2_co_block_status,
 
     .bdrv_co_preadv_part    = qcow2_co_preadv_part,
diff --git a/block/qed.c b/block/qed.c
index fb609cfba1..5da9726518 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -1675,7 +1675,6 @@ static BlockDriver bdrv_qed = {
     .bdrv_co_create           = bdrv_qed_co_create,
     .bdrv_co_create_opts      = bdrv_qed_co_create_opts,
     .bdrv_has_zero_init       = bdrv_has_zero_init_1,
-    .bdrv_has_zero_init_truncate = bdrv_has_zero_init_1,
     .bdrv_co_block_status     = bdrv_qed_co_block_status,
     .bdrv_co_readv            = bdrv_qed_co_readv,
     .bdrv_co_writev           = bdrv_qed_co_writev,
diff --git a/block/raw-format.c b/block/raw-format.c
index 351f2d91c6..9108e43696 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -414,11 +414,6 @@ static int raw_has_zero_init(BlockDriverState *bs)
     return bdrv_has_zero_init(bs->file->bs);
 }
 
-static int raw_has_zero_init_truncate(BlockDriverState *bs)
-{
-    return bdrv_has_zero_init_truncate(bs->file->bs);
-}
-
 static int coroutine_fn raw_co_create_opts(BlockDriver *drv,
                                            const char *filename,
                                            QemuOpts *opts,
@@ -582,7 +577,6 @@ BlockDriver bdrv_raw = {
     .bdrv_co_ioctl        = &raw_co_ioctl,
     .create_opts          = &raw_create_opts,
     .bdrv_has_zero_init   = &raw_has_zero_init,
-    .bdrv_has_zero_init_truncate = &raw_has_zero_init_truncate,
     .strong_runtime_opts  = raw_strong_runtime_opts,
     .mutable_opts         = mutable_opts,
 };
diff --git a/block/rbd.c b/block/rbd.c
index 331c45adb2..617553b022 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -1313,7 +1313,6 @@ static BlockDriver bdrv_rbd = {
     .bdrv_co_create         = qemu_rbd_co_create,
     .bdrv_co_create_opts    = qemu_rbd_co_create_opts,
     .bdrv_has_zero_init     = bdrv_has_zero_init_1,
-    .bdrv_has_zero_init_truncate = bdrv_has_zero_init_1,
     .bdrv_get_info          = qemu_rbd_getinfo,
     .create_opts            = &qemu_rbd_create_opts,
     .bdrv_getlength         = qemu_rbd_getlength,
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 739e6dee30..27a30d17f4 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -3226,7 +3226,6 @@ static BlockDriver bdrv_sheepdog = {
     .bdrv_co_create               = sd_co_create,
     .bdrv_co_create_opts          = sd_co_create_opts,
     .bdrv_has_zero_init           = bdrv_has_zero_init_1,
-    .bdrv_has_zero_init_truncate  = bdrv_has_zero_init_1,
     .bdrv_getlength               = sd_getlength,
     .bdrv_get_allocated_file_size = sd_get_allocated_file_size,
     .bdrv_co_truncate             = sd_co_truncate,
@@ -3265,7 +3264,6 @@ static BlockDriver bdrv_sheepdog_tcp = {
     .bdrv_co_create               = sd_co_create,
     .bdrv_co_create_opts          = sd_co_create_opts,
     .bdrv_has_zero_init           = bdrv_has_zero_init_1,
-    .bdrv_has_zero_init_truncate  = bdrv_has_zero_init_1,
     .bdrv_getlength               = sd_getlength,
     .bdrv_get_allocated_file_size = sd_get_allocated_file_size,
     .bdrv_co_truncate             = sd_co_truncate,
@@ -3304,7 +3302,6 @@ static BlockDriver bdrv_sheepdog_unix = {
     .bdrv_co_create               = sd_co_create,
     .bdrv_co_create_opts          = sd_co_create_opts,
     .bdrv_has_zero_init           = bdrv_has_zero_init_1,
-    .bdrv_has_zero_init_truncate  = bdrv_has_zero_init_1,
     .bdrv_getlength               = sd_getlength,
     .bdrv_get_allocated_file_size = sd_get_allocated_file_size,
     .bdrv_co_truncate             = sd_co_truncate,
diff --git a/block/ssh.c b/block/ssh.c
index f9e08a4900..098dbe03c1 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -1397,7 +1397,6 @@ static BlockDriver bdrv_ssh = {
     .bdrv_co_create_opts          = ssh_co_create_opts,
     .bdrv_close                   = ssh_close,
     .bdrv_has_zero_init           = ssh_has_zero_init,
-    .bdrv_has_zero_init_truncate  = ssh_has_zero_init,
     .bdrv_co_readv                = ssh_co_readv,
     .bdrv_co_writev               = ssh_co_writev,
     .bdrv_getlength               = ssh_getlength,
-- 
2.25.3

Re: [PULL 00/30] Block layer patches

[Peter Maydell]

On Fri, 8 May 2020 at 13:41, Kevin Wolf <kwolf@redhat.com> wrote:
>
> The following changes since commit 1b8c45899715d292398152ba97ef755ccaf84680:
>
>   Merge remote-tracking branch 'remotes/dgilbert/tags/pull-migration-20200507a' into staging (2020-05-07 18:43:20 +0100)
>
> are available in the Git repository at:
>
>   git://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to 47e0b38a13935cb666f88964c3096654092f42d6:
>
>   block: Drop unused .bdrv_has_zero_init_truncate (2020-05-08 13:26:35 +0200)
>
> ----------------------------------------------------------------
> Block layer patches:
>
> - qcow2: Fix preallocation on block devices
> - backup: Make sure that source and target size match
> - vmdk: Fix zero cluster handling
> - Follow-up cleanups and fixes for the truncate changes
> - iotests: Skip more tests if required drivers are missing
>

Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.1
for any user-visible changes.

-- PMM