Re: [PATCH v4 15/18] iotests/image-fleecing: add test case with bitmap

2022-02-24 Thread Vladimir Sementsov-Ogievskiy

24.02.2022 15:58, Hanna Reitz wrote:

On 16.02.22 20:46, Vladimir Sementsov-Ogievskiy wrote:

Note that reads zero areas (not dirty in the bitmap) fails, that's
correct.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  tests/qemu-iotests/tests/image-fleecing | 32 ++--
  tests/qemu-iotests/tests/image-fleecing.out | 84 +
  2 files changed, 108 insertions(+), 8 deletions(-)


Looks good, just one general usage question:


diff --git a/tests/qemu-iotests/tests/image-fleecing 
b/tests/qemu-iotests/tests/image-fleecing
index 909fc0a7ad..33995612be 100755
--- a/tests/qemu-iotests/tests/image-fleecing
+++ b/tests/qemu-iotests/tests/image-fleecing
@@ -23,7 +23,7 @@
  # Creator/Owner: John Snow 
  import iotests
-from iotests import log, qemu_img, qemu_io, qemu_io_silent
+from iotests import log, qemu_img, qemu_io, qemu_io_silent, 
qemu_io_pipe_and_status
  iotests.script_initialize(
  supported_fmts=['qcow2', 'qcow', 'qed', 'vmdk', 'vhdx', 'raw'],
@@ -50,11 +50,15 @@ remainder = [('0xd5', '0x108000',  '32k'), # Right-end of 
partial-left [1]
   ('0xcd', '0x3ff', '64k')] # patterns[3]
  def do_test(use_cbw, use_snapshot_access_filter, base_img_path,
-    fleece_img_path, nbd_sock_path, vm):
+    fleece_img_path, nbd_sock_path, vm,
+    bitmap=False):
  log('--- Setting up images ---')
  log('')
  assert qemu_img('create', '-f', iotests.imgfmt, base_img_path, '64M') == 0
+    if bitmap:
+    assert qemu_img('bitmap', '--add', base_img_path, 'bitmap0') == 0
+
  if use_snapshot_access_filter:
  assert use_cbw
  assert qemu_img('create', '-f', 'raw', fleece_img_path, '64M') == 0
@@ -106,12 +110,17 @@ def do_test(use_cbw, use_snapshot_access_filter, 
base_img_path,
  # Establish CBW from source to fleecing node
  if use_cbw:
-    log(vm.qmp('blockdev-add', {
+    fl_cbw = {
  'driver': 'copy-before-write',
  'node-name': 'fl-cbw',
  'file': src_node,
  'target': tmp_node
-    }))
+    }
+
+    if bitmap:
+    fl_cbw['bitmap'] = {'node': src_node, 'name': 'bitmap0'}
+
+    log(vm.qmp('blockdev-add', fl_cbw))
  log(vm.qmp('qom-set', path=qom_path, property='drive', 
value='fl-cbw'))


This makes me wonder how exactly the @bitmap parameter is to be used. In this 
case here, we use an active bitmap that tracks all writes, so it looks like a 
case of trying to copy the changes since some previous checkpoint (as a 
point-in-time state).  But if there are any writes between the blockdev-add and 
the qom-set, then they will not be included in the CBW bitmap.  Is that fine?  
Or is it perhaps even intentional?

(Is the idea that one would use a transaction to disable the current bitmap 
(say “A”), and add a new one (say “B”) at the same time, then use bitmap A for 
the CBW filter, delete it after the backup, and then use B for the subsequent 
backup?)



Hmm, good question. If we do this way, we break a point-in-time of backup.. 
We'll make a copy of disk in state of the moment of qom-set, but use an 
outdated copy of bitmap..

Good solution would do blockdev-add and qom-set in one transaction. But it's 
more possible to make transaction support for my proposed blockdev-replace, 
which should substitute qom-set in this scenario..

And supporting blockdev-add in transaction is not simple too.

With usual backup we simply do blockdev-backup and all needed bitmap 
manipulations in one transaction. With filter, actual backup start is qom-set 
(or blockdev-replace), not blockdev-add.. But we can't pass bitmap parameter to 
qom-set or blockdev-replace.

We probably could support blockdev-reopen in transaction, and change the bitmap 
in reopen.. But that seems wrong to me: we should not use reopen in scenario 
where we've just created this temporary node with all arguments we want.

Keeping in mind my recent series that introduces a kind of transaction for 
bdrv_close, may be the best and more native way is really support blockdev-add 
and blockdev-del in transaction.


The only alternative way I see is to not copy the user-given bitmap, but use 
exactly what user gives. This way, we do the following:

1. User give active bitmap A to cbw_open, so bitmap continue to track dirtiness.
2. User start a new dirty bitmap B
3. On filter insertion, we have a good bitmap with all needed dirty bits
4. After filter insertion, user stops tracking in bitmap A (disable it)

This way, we'll not lose any data. The drawback, is that we may copy some extra 
data, that was not actually dirty at point [3] (because we disable bitmap A 
after it, not in transaction). As well, bitmap B which will be used for next 
incremental backup will probably contain some extra dirty bits. That's not bad, 
but that's not an ideal architecture.

--
Best regards,
Vladimir



Re: [PATCH v4 15/18] iotests/image-fleecing: add test case with bitmap

2022-02-24 Thread Hanna Reitz

On 16.02.22 20:46, Vladimir Sementsov-Ogievskiy wrote:

Note that reads zero areas (not dirty in the bitmap) fails, that's
correct.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  tests/qemu-iotests/tests/image-fleecing | 32 ++--
  tests/qemu-iotests/tests/image-fleecing.out | 84 +
  2 files changed, 108 insertions(+), 8 deletions(-)


Looks good, just one general usage question:


diff --git a/tests/qemu-iotests/tests/image-fleecing 
b/tests/qemu-iotests/tests/image-fleecing
index 909fc0a7ad..33995612be 100755
--- a/tests/qemu-iotests/tests/image-fleecing
+++ b/tests/qemu-iotests/tests/image-fleecing
@@ -23,7 +23,7 @@
  # Creator/Owner: John Snow 
  
  import iotests

-from iotests import log, qemu_img, qemu_io, qemu_io_silent
+from iotests import log, qemu_img, qemu_io, qemu_io_silent, 
qemu_io_pipe_and_status
  
  iotests.script_initialize(

  supported_fmts=['qcow2', 'qcow', 'qed', 'vmdk', 'vhdx', 'raw'],
@@ -50,11 +50,15 @@ remainder = [('0xd5', '0x108000',  '32k'), # Right-end of 
partial-left [1]
   ('0xcd', '0x3ff', '64k')] # patterns[3]
  
  def do_test(use_cbw, use_snapshot_access_filter, base_img_path,

-fleece_img_path, nbd_sock_path, vm):
+fleece_img_path, nbd_sock_path, vm,
+bitmap=False):
  log('--- Setting up images ---')
  log('')
  
  assert qemu_img('create', '-f', iotests.imgfmt, base_img_path, '64M') == 0

+if bitmap:
+assert qemu_img('bitmap', '--add', base_img_path, 'bitmap0') == 0
+
  if use_snapshot_access_filter:
  assert use_cbw
  assert qemu_img('create', '-f', 'raw', fleece_img_path, '64M') == 0
@@ -106,12 +110,17 @@ def do_test(use_cbw, use_snapshot_access_filter, 
base_img_path,
  
  # Establish CBW from source to fleecing node

  if use_cbw:
-log(vm.qmp('blockdev-add', {
+fl_cbw = {
  'driver': 'copy-before-write',
  'node-name': 'fl-cbw',
  'file': src_node,
  'target': tmp_node
-}))
+}
+
+if bitmap:
+fl_cbw['bitmap'] = {'node': src_node, 'name': 'bitmap0'}
+
+log(vm.qmp('blockdev-add', fl_cbw))
  
  log(vm.qmp('qom-set', path=qom_path, property='drive', value='fl-cbw'))


This makes me wonder how exactly the @bitmap parameter is to be used.  
In this case here, we use an active bitmap that tracks all writes, so it 
looks like a case of trying to copy the changes since some previous 
checkpoint (as a point-in-time state).  But if there are any writes 
between the blockdev-add and the qom-set, then they will not be included 
in the CBW bitmap.  Is that fine?  Or is it perhaps even intentional?


(Is the idea that one would use a transaction to disable the current 
bitmap (say “A”), and add a new one (say “B”) at the same time, then use 
bitmap A for the CBW filter, delete it after the backup, and then use B 
for the subsequent backup?)





[PATCH v4 15/18] iotests/image-fleecing: add test case with bitmap

2022-02-16 Thread Vladimir Sementsov-Ogievskiy
Note that reads zero areas (not dirty in the bitmap) fails, that's
correct.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 tests/qemu-iotests/tests/image-fleecing | 32 ++--
 tests/qemu-iotests/tests/image-fleecing.out | 84 +
 2 files changed, 108 insertions(+), 8 deletions(-)

diff --git a/tests/qemu-iotests/tests/image-fleecing 
b/tests/qemu-iotests/tests/image-fleecing
index 909fc0a7ad..33995612be 100755
--- a/tests/qemu-iotests/tests/image-fleecing
+++ b/tests/qemu-iotests/tests/image-fleecing
@@ -23,7 +23,7 @@
 # Creator/Owner: John Snow 
 
 import iotests
-from iotests import log, qemu_img, qemu_io, qemu_io_silent
+from iotests import log, qemu_img, qemu_io, qemu_io_silent, 
qemu_io_pipe_and_status
 
 iotests.script_initialize(
 supported_fmts=['qcow2', 'qcow', 'qed', 'vmdk', 'vhdx', 'raw'],
@@ -50,11 +50,15 @@ remainder = [('0xd5', '0x108000',  '32k'), # Right-end of 
partial-left [1]
  ('0xcd', '0x3ff', '64k')] # patterns[3]
 
 def do_test(use_cbw, use_snapshot_access_filter, base_img_path,
-fleece_img_path, nbd_sock_path, vm):
+fleece_img_path, nbd_sock_path, vm,
+bitmap=False):
 log('--- Setting up images ---')
 log('')
 
 assert qemu_img('create', '-f', iotests.imgfmt, base_img_path, '64M') == 0
+if bitmap:
+assert qemu_img('bitmap', '--add', base_img_path, 'bitmap0') == 0
+
 if use_snapshot_access_filter:
 assert use_cbw
 assert qemu_img('create', '-f', 'raw', fleece_img_path, '64M') == 0
@@ -106,12 +110,17 @@ def do_test(use_cbw, use_snapshot_access_filter, 
base_img_path,
 
 # Establish CBW from source to fleecing node
 if use_cbw:
-log(vm.qmp('blockdev-add', {
+fl_cbw = {
 'driver': 'copy-before-write',
 'node-name': 'fl-cbw',
 'file': src_node,
 'target': tmp_node
-}))
+}
+
+if bitmap:
+fl_cbw['bitmap'] = {'node': src_node, 'name': 'bitmap0'}
+
+log(vm.qmp('blockdev-add', fl_cbw))
 
 log(vm.qmp('qom-set', path=qom_path, property='drive', value='fl-cbw'))
 
@@ -148,7 +157,9 @@ def do_test(use_cbw, use_snapshot_access_filter, 
base_img_path,
 for p in patterns + zeroes:
 cmd = 'read -P%s %s %s' % p
 log(cmd)
-assert qemu_io_silent('-r', '-f', 'raw', '-c', cmd, nbd_uri) == 0
+out, ret = qemu_io_pipe_and_status('-r', '-f', 'raw', '-c', cmd, 
nbd_uri)
+if ret != 0:
+print(out)
 
 log('')
 log('--- Testing COW ---')
@@ -166,7 +177,9 @@ def do_test(use_cbw, use_snapshot_access_filter, 
base_img_path,
 for p in patterns + zeroes:
 cmd = 'read -P%s %s %s' % p
 log(cmd)
-assert qemu_io_silent('-r', '-f', 'raw', '-c', cmd, nbd_uri) == 0
+out, ret = qemu_io_pipe_and_status('-r', '-f', 'raw', '-c', cmd, 
nbd_uri)
+if ret != 0:
+print(out)
 
 log('')
 log('--- Cleanup ---')
@@ -201,14 +214,14 @@ def do_test(use_cbw, use_snapshot_access_filter, 
base_img_path,
 log('Done')
 
 
-def test(use_cbw, use_snapshot_access_filter):
+def test(use_cbw, use_snapshot_access_filter, bitmap=False):
 with iotests.FilePath('base.img') as base_img_path, \
  iotests.FilePath('fleece.img') as fleece_img_path, \
  iotests.FilePath('nbd.sock',
   base_dir=iotests.sock_dir) as nbd_sock_path, \
  iotests.VM() as vm:
 do_test(use_cbw, use_snapshot_access_filter, base_img_path,
-fleece_img_path, nbd_sock_path, vm)
+fleece_img_path, nbd_sock_path, vm, bitmap=bitmap)
 
 
 log('=== Test backup(sync=none) based fleecing ===\n')
@@ -219,3 +232,6 @@ test(True, False)
 
 log('=== Test fleecing-format based fleecing ===\n')
 test(True, True)
+
+log('=== Test fleecing-format based fleecing with bitmap ===\n')
+test(True, True, bitmap=True)
diff --git a/tests/qemu-iotests/tests/image-fleecing.out 
b/tests/qemu-iotests/tests/image-fleecing.out
index da0af93388..62e1c1fe42 100644
--- a/tests/qemu-iotests/tests/image-fleecing.out
+++ b/tests/qemu-iotests/tests/image-fleecing.out
@@ -190,6 +190,90 @@ read -P0 0x00f8000 32k
 read -P0 0x201 32k
 read -P0 0x3fe 64k
 
+--- Cleanup ---
+
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"return": {}}
+
+--- Confirming writes ---
+
+read -P0xab 0 64k
+read -P0xad 0x00f8000 64k
+read -P0x1d 0x2008000 64k
+read -P0xea 0x3fe 64k
+read -P0xd5 0x108000 32k
+read -P0xdc 32M 32k
+read -P0xcd 0x3ff 64k
+
+Done
+=== Test fleecing-format based fleecing with bitmap ===
+
+--- Setting up images ---
+
+Done
+
+--- Launching VM ---
+
+Done
+
+--- Setting up Fleecing Graph ---
+
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"return": {}}
+
+--- Setting up NBD Export ---
+
+{"return": {}}
+{"return": {}}
+
+--- Sanity Check ---
+
+read -P0x5d 0 64k
+read -P0xd5 1M 64k
+read -P0xdc 32M 64k
+read -P0xcd