On 31.08.19 13:41, Vladimir Sementsov-Ogievskiy wrote: > 09.08.2019 19:14, Max Reitz wrote: >> This patch adds some tests on how commit copes with filter nodes. >> >> Signed-off-by: Max Reitz <[email protected]> >> --- >> tests/qemu-iotests/040 | 177 +++++++++++++++++++++++++++++++++++++ >> tests/qemu-iotests/040.out | 4 +- >> 2 files changed, 179 insertions(+), 2 deletions(-) >> >> diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040 >> index 6db9abf8e6..a0a0db8889 100755 >> --- a/tests/qemu-iotests/040 >> +++ b/tests/qemu-iotests/040
[...]
>> + def tearDown(self):
>> + self.vm.shutdown(has_quit=self.has_quit)
>> +
>> + for index in range(len(self.pattern_files)):
>
> you may use enumerate for such cases:
> for ind, file in enumerate(self.pattern_files):
> ...
Ah, nice.
>> + result = qemu_io('-f', iotests.imgfmt,
>> + '-c', 'read -P %i %iM 1M' % (index + 1, index),
>> + self.pattern_files[index])
>> + self.assertFalse('Pattern verification failed' in result)
>
> A bit better would be to keep this loop in a function and do "writes" through
> it too,
> to make it more obvious that they are the same.. But I'm OK with it as is.
Hm, yes. I’ll have a look.
>> +
>> + os.remove(self.img3)
>> + os.remove(self.img2)
>> + os.remove(self.img1)
>> + os.remove(self.img0)
>> +
>> + # Filters make for funny filenames, so we cannot just use
>> + # self.imgX to get them
>> + def get_filename(self, node):
>> + return self.vm.node_info(node)['image']['filename']
>> +
>
> maybe:
> def assertHasNode(self, node_name):
> self.assertIsNotNone(self.vm.node_info(node_name))
>
> and similar for assertNoNode...
Hm, I don’t know. It fits on one line either way.
>> + def test_filterless_commit(self):
>> + self.assert_no_active_block_jobs()
>
> why not just to include this call into setUp() ? Or even, just drop it?
> We create and start new vm in setUp, it don't have any block jobs for sure.
Other tests do it the same way, e.g. 030, 040, and 041.
[...]
>> + self.assertIsNone(self.vm.node_info('top-filter'))
>> + self.assertIsNone(self.vm.node_info('cow-3'))
>> + self.assertIsNotNone(self.vm.node_info('cow-2'))
>
> It would be good to assert here the cow-2 became drv0 child. However,
> otherwise
> it should be automatically dropped, so it's not necessary.
Yep, like cow-3. I’ll look into it anyway.
>> +
>> + # 3 has been comitted into 2
>> + self.pattern_files[3] = self.img2
>> +
>> + def test_filtered_active_commit_without_filter(self):
>> + self.assert_no_active_block_jobs()
>> + result = self.vm.qmp('block-commit',
>> + job_id='commit',
>> + device='top-filter',
>> + top_node='cow-3',
>> + base_node='cow-2')
>> + self.assert_qmp(result, 'return', {})
>
> can we check that really "active" commit is started, i.e. mirror block job?
We do:
>> + self.complete_and_wait(drive='commit')
wait_ready is True by default, so this will first wait for a READY
event. That only happens for active commit.
Max
signature.asc
Description: OpenPGP digital signature
