| Message ID | 20220228113927.1852146-17-vsementsov@virtuozzo.com |
|---|---|
| State | New |
| Headers | show |
| Series | Make image fleecing more usable | expand |
On 28.02.22 12:39, Vladimir Sementsov-Ogievskiy wrote: > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > tests/qemu-iotests/tests/image-fleecing | 120 ++++++++++++++------ > tests/qemu-iotests/tests/image-fleecing.out | 63 ++++++++++ > 2 files changed, 151 insertions(+), 32 deletions(-) > > diff --git a/tests/qemu-iotests/tests/image-fleecing b/tests/qemu-iotests/tests/image-fleecing > index 33995612be..89c79af698 100755 > --- a/tests/qemu-iotests/tests/image-fleecing > +++ b/tests/qemu-iotests/tests/image-fleecing [...] > @@ -170,6 +196,20 @@ def do_test(use_cbw, use_snapshot_access_filter, base_img_path, > log(cmd) > log(vm.hmp_qemu_io(qom_path, cmd, qdev=True)) > > + if push_backup: > + # Check that previous operations were done during backup, not after > + result = vm.qmp('query-block-jobs') > + if len(result['return']) != 1: > + log('Backup finished too fast, COW is not tested') I don’t understand why this log is here, its message sounds like “case not run”, but first this logged message will make the whole test fail... > + > + result = vm.qmp('block-job-set-speed', device='push-backup', speed=0) > + assert result == {'return': {}} ...and then this will fail, too. Either this is a hard failure, then the log shouldn’t include “COW is not tested” (because it is tested, and the case has failed); or it’s a casenotrun, and then nothing should be logged (the message should be appended to .casenotrun), and the block-job-set-speed call and waiting for BLOCK_JOB_COMPLETED should only be done when the job is still in the job list. > + > + log(vm.event_wait(name='BLOCK_JOB_COMPLETED', > + match={'data': {'device': 'push-backup'}}), > + filters=[iotests.filter_qmp_event]) > + log(vm.qmp('blockdev-del', node_name='target')) > + > log('') > log('--- Verifying Data ---') > log('') > @@ -177,15 +217,19 @@ 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) > - out, ret = qemu_io_pipe_and_status('-r', '-f', 'raw', '-c', cmd, nbd_uri) > - if ret != 0: > - print(out) > + if push_backup: > + assert qemu_io_silent('-r', '-c', cmd, target_img_path) == 0 > + else: > + out, ret = qemu_io_pipe_and_status('-r', '-f', 'raw', '-c', cmd, nbd_uri) > + if ret != 0: > + print(out) The existing principle of “print qemu-io’s output on error” seemed perfectly fine to me. Why not continue using it? (e.g. like args = ['-r', '-c', cmd] if push_backup: args += [target_img_path] else: args += ['-f', 'raw', nbd_uri] out, ret = qemu_io_pipe_and_status(*args) ) > log('') > log('--- Cleanup ---') > log('') > > - log(vm.qmp('nbd-server-stop')) > + if not push_backup: > + log(vm.qmp('nbd-server-stop')) > > if use_cbw: > if use_snapshot_access_filter: > +read -P0xcd 0x3ff0000 64k > + > +Done
03.03.2022 13:58, Hanna Reitz wrote: > On 28.02.22 12:39, Vladimir Sementsov-Ogievskiy wrote: >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >> --- >> tests/qemu-iotests/tests/image-fleecing | 120 ++++++++++++++------ >> tests/qemu-iotests/tests/image-fleecing.out | 63 ++++++++++ >> 2 files changed, 151 insertions(+), 32 deletions(-) >> >> diff --git a/tests/qemu-iotests/tests/image-fleecing b/tests/qemu-iotests/tests/image-fleecing >> index 33995612be..89c79af698 100755 >> --- a/tests/qemu-iotests/tests/image-fleecing >> +++ b/tests/qemu-iotests/tests/image-fleecing > > [...] > >> @@ -170,6 +196,20 @@ def do_test(use_cbw, use_snapshot_access_filter, base_img_path, >> log(cmd) >> log(vm.hmp_qemu_io(qom_path, cmd, qdev=True)) >> + if push_backup: >> + # Check that previous operations were done during backup, not after >> + result = vm.qmp('query-block-jobs') >> + if len(result['return']) != 1: >> + log('Backup finished too fast, COW is not tested') > > I don’t understand why this log is here, its message sounds like “case not run”, but first this logged message will make the whole test fail... This log means that test doesn't test what it should. If that happens, we'll need to adjust disk size, backup speed, or something like this. I hope, that will not happen, at least it works for me ) > >> + >> + result = vm.qmp('block-job-set-speed', device='push-backup', speed=0) >> + assert result == {'return': {}} > > ...and then this will fail, too. > > Either this is a hard failure, then the log shouldn’t include “COW is not tested” (because it is tested, and the case has failed); or it’s a casenotrun, and then nothing should be logged (the message should be appended to .casenotrun), and the block-job-set-speed call and waiting for BLOCK_JOB_COMPLETED should only be done when the job is still in the job list. OK, I understand. What about this: # Check that backup is not finished yet. If it is, it's possible that backup # finished even before guest write, and we didn't actually test # copy-before-write operation. If this happen, we'll need to adjust storage # size or backup speed or something like this. assert len(result['return'] == 1 > >> + >> + log(vm.event_wait(name='BLOCK_JOB_COMPLETED', >> + match={'data': {'device': 'push-backup'}}), >> + filters=[iotests.filter_qmp_event]) >> + log(vm.qmp('blockdev-del', node_name='target')) >> + >> log('') >> log('--- Verifying Data ---') >> log('') >> @@ -177,15 +217,19 @@ 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) >> - out, ret = qemu_io_pipe_and_status('-r', '-f', 'raw', '-c', cmd, nbd_uri) >> - if ret != 0: >> - print(out) >> + if push_backup: >> + assert qemu_io_silent('-r', '-c', cmd, target_img_path) == 0 >> + else: >> + out, ret = qemu_io_pipe_and_status('-r', '-f', 'raw', '-c', cmd, nbd_uri) >> + if ret != 0: >> + print(out) > > The existing principle of “print qemu-io’s output on error” seemed perfectly fine to me. Why not continue using it? > > (e.g. like > > args = ['-r', '-c', cmd] > if push_backup: > args += [target_img_path] > else: > args += ['-f', 'raw', nbd_uri] > out, ret = qemu_io_pipe_and_status(*args) > > ) I don't remember why did I changed it. Your variant seems good. > >> log('') >> log('--- Cleanup ---') >> log('') >> - log(vm.qmp('nbd-server-stop')) >> + if not push_backup: >> + log(vm.qmp('nbd-server-stop')) >> if use_cbw: >> if use_snapshot_access_filter: >> +read -P0xcd 0x3ff0000 64k >> + >> +Done >
diff --git a/tests/qemu-iotests/tests/image-fleecing b/tests/qemu-iotests/tests/image-fleecing index 33995612be..89c79af698 100755 --- a/tests/qemu-iotests/tests/image-fleecing +++ b/tests/qemu-iotests/tests/image-fleecing @@ -49,9 +49,15 @@ remainder = [('0xd5', '0x108000', '32k'), # Right-end of partial-left [1] ('0xdc', '32M', '32k'), # Left-end of partial-right [2] ('0xcd', '0x3ff0000', '64k')] # patterns[3] -def do_test(use_cbw, use_snapshot_access_filter, base_img_path, - fleece_img_path, nbd_sock_path, vm, +def do_test(vm, use_cbw, use_snapshot_access_filter, base_img_path, + fleece_img_path, nbd_sock_path=None, + target_img_path=None, bitmap=False): + push_backup = target_img_path is not None + assert (nbd_sock_path is not None) != push_backup + if push_backup: + assert use_cbw + log('--- Setting up images ---') log('') @@ -65,6 +71,9 @@ def do_test(use_cbw, use_snapshot_access_filter, base_img_path, else: assert qemu_img('create', '-f', 'qcow2', fleece_img_path, '64M') == 0 + if push_backup: + assert qemu_img('create', '-f', 'qcow2', target_img_path, '64M') == 0 + for p in patterns: qemu_io('-f', iotests.imgfmt, '-c', 'write -P%s %s %s' % p, base_img_path) @@ -139,27 +148,44 @@ def do_test(use_cbw, use_snapshot_access_filter, base_img_path, export_node = 'fl-access' if use_snapshot_access_filter else tmp_node - log('') - log('--- Setting up NBD Export ---') - log('') + if push_backup: + log('') + log('--- Starting actual backup ---') + log('') - nbd_uri = 'nbd+unix:///%s?socket=%s' % (export_node, nbd_sock_path) - log(vm.qmp('nbd-server-start', - {'addr': {'type': 'unix', - 'data': {'path': nbd_sock_path}}})) + log(vm.qmp('blockdev-add', **{ + 'driver': iotests.imgfmt, + 'node-name': 'target', + 'file': { + 'driver': 'file', + 'filename': target_img_path + } + })) + log(vm.qmp('blockdev-backup', device=export_node, + sync='full', target='target', + job_id='push-backup', speed=1)) + else: + log('') + log('--- Setting up NBD Export ---') + log('') - log(vm.qmp('nbd-server-add', device=export_node)) + nbd_uri = 'nbd+unix:///%s?socket=%s' % (export_node, nbd_sock_path) + log(vm.qmp('nbd-server-start', + {'addr': { 'type': 'unix', + 'data': { 'path': nbd_sock_path } } })) - log('') - log('--- Sanity Check ---') - log('') + log(vm.qmp('nbd-server-add', device=export_node)) - for p in patterns + zeroes: - cmd = 'read -P%s %s %s' % p - log(cmd) - out, ret = qemu_io_pipe_and_status('-r', '-f', 'raw', '-c', cmd, nbd_uri) - if ret != 0: - print(out) + log('') + log('--- Sanity Check ---') + log('') + + for p in patterns + zeroes: + cmd = 'read -P%s %s %s' % p + log(cmd) + out, ret = qemu_io_pipe_and_status('-r', '-f', 'raw', '-c', cmd, nbd_uri) + if ret != 0: + print(out) log('') log('--- Testing COW ---') @@ -170,6 +196,20 @@ def do_test(use_cbw, use_snapshot_access_filter, base_img_path, log(cmd) log(vm.hmp_qemu_io(qom_path, cmd, qdev=True)) + if push_backup: + # Check that previous operations were done during backup, not after + result = vm.qmp('query-block-jobs') + if len(result['return']) != 1: + log('Backup finished too fast, COW is not tested') + + result = vm.qmp('block-job-set-speed', device='push-backup', speed=0) + assert result == {'return': {}} + + log(vm.event_wait(name='BLOCK_JOB_COMPLETED', + match={'data': {'device': 'push-backup'}}), + filters=[iotests.filter_qmp_event]) + log(vm.qmp('blockdev-del', node_name='target')) + log('') log('--- Verifying Data ---') log('') @@ -177,15 +217,19 @@ 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) - out, ret = qemu_io_pipe_and_status('-r', '-f', 'raw', '-c', cmd, nbd_uri) - if ret != 0: - print(out) + if push_backup: + assert qemu_io_silent('-r', '-c', cmd, target_img_path) == 0 + else: + out, ret = qemu_io_pipe_and_status('-r', '-f', 'raw', '-c', cmd, nbd_uri) + if ret != 0: + print(out) log('') log('--- Cleanup ---') log('') - log(vm.qmp('nbd-server-stop')) + if not push_backup: + log(vm.qmp('nbd-server-stop')) if use_cbw: if use_snapshot_access_filter: @@ -214,24 +258,36 @@ def do_test(use_cbw, use_snapshot_access_filter, base_img_path, log('Done') -def test(use_cbw, use_snapshot_access_filter, bitmap=False): +def test(use_cbw, use_snapshot_access_filter, + nbd_sock_path=None, target_img_path=None, 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, bitmap=bitmap) + do_test(vm, use_cbw, use_snapshot_access_filter, base_img_path, + fleece_img_path, nbd_sock_path, target_img_path, + bitmap=bitmap) + +def test_pull(use_cbw, use_snapshot_access_filter, bitmap=False): + with iotests.FilePath('nbd.sock', + base_dir=iotests.sock_dir) as nbd_sock_path: + test(use_cbw, use_snapshot_access_filter, nbd_sock_path, None, bitmap=bitmap) + +def test_push(): + with iotests.FilePath('target.img') as target_img_path: + test(True, True, None, target_img_path) log('=== Test backup(sync=none) based fleecing ===\n') -test(False, False) +test_pull(False, False) log('=== Test cbw-filter based fleecing ===\n') -test(True, False) +test_pull(True, False) log('=== Test fleecing-format based fleecing ===\n') -test(True, True) +test_pull(True, True) log('=== Test fleecing-format based fleecing with bitmap ===\n') -test(True, True, bitmap=True) +test_pull(True, True, bitmap=True) + +log('=== Test push backup with fleecing ===\n') +test_push() diff --git a/tests/qemu-iotests/tests/image-fleecing.out b/tests/qemu-iotests/tests/image-fleecing.out index 62e1c1fe42..acfc89ff0e 100644 --- a/tests/qemu-iotests/tests/image-fleecing.out +++ b/tests/qemu-iotests/tests/image-fleecing.out @@ -293,3 +293,66 @@ read -P0xdc 32M 32k read -P0xcd 0x3ff0000 64k Done +=== Test push backup with fleecing === + +--- Setting up images --- + +Done + +--- Launching VM --- + +Done + +--- Setting up Fleecing Graph --- + +{"return": {}} +{"return": {}} +{"return": {}} +{"return": {}} + +--- Starting actual backup --- + +{"return": {}} +{"return": {}} + +--- Testing COW --- + +write -P0xab 0 64k +{"return": ""} +write -P0xad 0x00f8000 64k +{"return": ""} +write -P0x1d 0x2008000 64k +{"return": ""} +write -P0xea 0x3fe0000 64k +{"return": ""} +{"data": {"device": "push-backup", "len": 67108864, "offset": 67108864, "speed": 0, "type": "backup"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}} +{"return": {}} + +--- Verifying Data --- + +read -P0x5d 0 64k +read -P0xd5 1M 64k +read -P0xdc 32M 64k +read -P0xcd 0x3ff0000 64k +read -P0 0x00f8000 32k +read -P0 0x2010000 32k +read -P0 0x3fe0000 64k + +--- Cleanup --- + +{"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
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- tests/qemu-iotests/tests/image-fleecing | 120 ++++++++++++++------ tests/qemu-iotests/tests/image-fleecing.out | 63 ++++++++++ 2 files changed, 151 insertions(+), 32 deletions(-)