| Message ID | 20220216194617.126484-16-vsementsov@virtuozzo.com |
|---|---|
| State | New |
| Headers | show |
| Series | Make image fleecing more usable | expand |
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 <vsementsov@virtuozzo.com> > --- > 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 <jsnow@redhat.com> > > 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', '0x3ff0000', '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?)
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 <vsementsov@virtuozzo.com> >> --- >> 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 <jsnow@redhat.com> >> 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', '0x3ff0000', '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.
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 <jsnow@redhat.com> 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', '0x3ff0000', '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 0x2010000 32k read -P0 0x3fe0000 64k +--- Cleanup --- + +{"return": {}} +{"return": {}} +{"return": {}} +{"return": {}} +{"return": {}} + +--- Confirming writes --- + +read -P0xab 0 64k +read -P0xad 0x00f8000 64k +read -P0x1d 0x2008000 64k +read -P0xea 0x3fe0000 64k +read -P0xd5 0x108000 32k +read -P0xdc 32M 32k +read -P0xcd 0x3ff0000 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 0x3ff0000 64k +read -P0 0x00f8000 32k +read failed: Invalid argument + +read -P0 0x2010000 32k +read failed: Invalid argument + +read -P0 0x3fe0000 64k +read failed: Invalid argument + + +--- Testing COW --- + +write -P0xab 0 64k +{"return": ""} +write -P0xad 0x00f8000 64k +{"return": ""} +write -P0x1d 0x2008000 64k +{"return": ""} +write -P0xea 0x3fe0000 64k +{"return": ""} + +--- Verifying Data --- + +read -P0x5d 0 64k +read -P0xd5 1M 64k +read -P0xdc 32M 64k +read -P0xcd 0x3ff0000 64k +read -P0 0x00f8000 32k +read failed: Invalid argument + +read -P0 0x2010000 32k +read failed: Invalid argument + +read -P0 0x3fe0000 64k +read failed: Invalid argument + + --- Cleanup --- {"return": {}}
Note that reads zero areas (not dirty in the bitmap) fails, that's correct. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- tests/qemu-iotests/tests/image-fleecing | 32 ++++++-- tests/qemu-iotests/tests/image-fleecing.out | 84 +++++++++++++++++++++ 2 files changed, 108 insertions(+), 8 deletions(-)