[PATCH 0/2] fix backup-discard-source test for XFS

[PATCH 0/2] fix backup-discard-source test for XFS

[Vladimir Sementsov-Ogievskiy]

Hi all!

As Kevin reported, the test doesn't work on XFS, as it rely on disk
usage.

Fix it, switching to dirty bitmap for guest write tracking.

Vladimir Sementsov-Ogievskiy (2):
  iotests/backup-discard-source: convert size variable to be int
  iotests/backup-discard-source: don't use actual-size

 .../qemu-iotests/tests/backup-discard-source  | 39 ++++++++++++-------
 1 file changed, 25 insertions(+), 14 deletions(-)

-- 
2.34.1

[PATCH 1/2] iotests/backup-discard-source: convert size variable to be int

[Vladimir Sementsov-Ogievskiy]

Make variable reusable in code for checks. Don't care to change "512 *
1024" invocations as they will be dropped in the next commit.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 tests/qemu-iotests/tests/backup-discard-source | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tests/qemu-iotests/tests/backup-discard-source b/tests/qemu-iotests/tests/backup-discard-source
index 2391b12acd..05fbe5d26b 100755
--- a/tests/qemu-iotests/tests/backup-discard-source
+++ b/tests/qemu-iotests/tests/backup-discard-source
@@ -28,7 +28,7 @@ from iotests import qemu_img_create, qemu_img_map, qemu_io
 temp_img = os.path.join(iotests.test_dir, 'temp')
 source_img = os.path.join(iotests.test_dir, 'source')
 target_img = os.path.join(iotests.test_dir, 'target')
-size = '1M'
+size = 1024 * 1024
 
 
 def get_actual_size(vm, node_name):
@@ -39,9 +39,9 @@ def get_actual_size(vm, node_name):
 
 class TestBackup(iotests.QMPTestCase):
     def setUp(self):
-        qemu_img_create('-f', iotests.imgfmt, source_img, size)
-        qemu_img_create('-f', iotests.imgfmt, temp_img, size)
-        qemu_img_create('-f', iotests.imgfmt, target_img, size)
+        qemu_img_create('-f', iotests.imgfmt, source_img, str(size))
+        qemu_img_create('-f', iotests.imgfmt, temp_img, str(size))
+        qemu_img_create('-f', iotests.imgfmt, target_img, str(size))
         qemu_io('-c', 'write 0 1M', source_img)
 
         self.vm = iotests.VM()
@@ -98,7 +98,7 @@ class TestBackup(iotests.QMPTestCase):
         mapping = qemu_img_map(temp_img)
         self.assertEqual(len(mapping), 1)
         self.assertEqual(mapping[0]['start'], 0)
-        self.assertEqual(mapping[0]['length'], 1024 * 1024)
+        self.assertEqual(mapping[0]['length'], size)
         self.assertEqual(mapping[0]['data'], False)
 
         os.remove(temp_img)
@@ -125,7 +125,7 @@ class TestBackup(iotests.QMPTestCase):
         self.assert_qmp(result, 'return', '')
 
         # Check that data is written to temporary image
-        self.assertGreater(get_actual_size(self.vm, 'temp'), 1024 * 1024)
+        self.assertGreater(get_actual_size(self.vm, 'temp'), size)
 
         self.do_backup()
 
-- 
2.34.1

[PATCH 2/2] iotests/backup-discard-source: don't use actual-size

[Vladimir Sementsov-Ogievskiy]

Relying on disk usage is bad thing, and test just doesn't work on XFS.

Let's instead add a dirty bitmap to track writes to test image.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 .../qemu-iotests/tests/backup-discard-source  | 29 +++++++++++++------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/tests/qemu-iotests/tests/backup-discard-source b/tests/qemu-iotests/tests/backup-discard-source
index 05fbe5d26b..17fef9c6d3 100755
--- a/tests/qemu-iotests/tests/backup-discard-source
+++ b/tests/qemu-iotests/tests/backup-discard-source
@@ -31,12 +31,6 @@ target_img = os.path.join(iotests.test_dir, 'target')
 size = 1024 * 1024
 
 
-def get_actual_size(vm, node_name):
-    nodes = vm.cmd('query-named-block-nodes', flat=True)
-    node = next(n for n in nodes if n['node-name'] == node_name)
-    return node['image']['actual-size']
-
-
 class TestBackup(iotests.QMPTestCase):
     def setUp(self):
         qemu_img_create('-f', iotests.imgfmt, source_img, str(size))
@@ -84,7 +78,12 @@ class TestBackup(iotests.QMPTestCase):
             }
         })
 
-        self.assertLess(get_actual_size(self.vm, 'temp'), 512 * 1024)
+        self.bitmap = {
+            'node': 'temp',
+            'name': 'bitmap0'
+        }
+
+        self.vm.cmd('block-dirty-bitmap-add', self.bitmap)
 
     def tearDown(self):
         # That should fail, because region is discarded
@@ -113,6 +112,13 @@ class TestBackup(iotests.QMPTestCase):
 
         self.vm.event_wait(name='BLOCK_JOB_COMPLETED')
 
+    def get_bitmap_count(self):
+        nodes = self.vm.cmd('query-named-block-nodes', flat=True)
+        temp = next(n for n in nodes if n['node-name'] == 'temp')
+        bitmap = temp['dirty-bitmaps'][0]
+        assert bitmap['name'] == self.bitmap['name']
+        return bitmap['count']
+
     def test_discard_written(self):
         """
         1. Guest writes
@@ -125,7 +131,7 @@ class TestBackup(iotests.QMPTestCase):
         self.assert_qmp(result, 'return', '')
 
         # Check that data is written to temporary image
-        self.assertGreater(get_actual_size(self.vm, 'temp'), size)
+        self.assertEqual(self.get_bitmap_count(), size)
 
         self.do_backup()
 
@@ -138,13 +144,18 @@ class TestBackup(iotests.QMPTestCase):
         """
         self.do_backup()
 
+        # backup job did discard operation and pollute the bitmap,
+        # we have to clean the bitmap, to check next write
+        self.assertEqual(self.get_bitmap_count(), size)
+        self.vm.cmd('block-dirty-bitmap-clear', self.bitmap)
+
         # Try trigger copy-before-write operation
         result = self.vm.hmp_qemu_io('cbw', 'write 0 1M')
         self.assert_qmp(result, 'return', '')
 
         # Check that data is not written to temporary image, as region
         # is discarded from copy-before-write process
-        self.assertLess(get_actual_size(self.vm, 'temp'), 512 * 1024)
+        self.assertEqual(self.get_bitmap_count(), 0)
 
 
 if __name__ == '__main__':
-- 
2.34.1

Re: [PATCH 0/2] fix backup-discard-source test for XFS

[Vladimir Sementsov-Ogievskiy]

Hi Kevin!

Now I revisit my old series, and looking here I see that I forget add you into CC.

Does it still make sense?

On 20.06.24 17:44, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
> 
> As Kevin reported, the test doesn't work on XFS, as it rely on disk
> usage.
> 
> Fix it, switching to dirty bitmap for guest write tracking.
> 
> Vladimir Sementsov-Ogievskiy (2):
>    iotests/backup-discard-source: convert size variable to be int
>    iotests/backup-discard-source: don't use actual-size
> 
>   .../qemu-iotests/tests/backup-discard-source  | 39 ++++++++++++-------
>   1 file changed, 25 insertions(+), 14 deletions(-)
> 

-- 
Best regards,
Vladimir

Re: [PATCH 0/2] fix backup-discard-source test for XFS

[Kevin Wolf]

Am 03.10.2024 um 08:39 hat Vladimir Sementsov-Ogievskiy geschrieben:
> Hi Kevin!
> 
> Now I revisit my old series, and looking here I see that I forget add you into CC.
> 
> Does it still make sense?

Oops, I missed this one indeed.

Thanks, applied to the block branch.

Kevin