[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v6 39/42] iotests: Add filter commit test cases
From: |
Max Reitz |
Subject: |
Re: [Qemu-devel] [PATCH v6 39/42] iotests: Add filter commit test cases |
Date: |
Mon, 2 Sep 2019 17:06:56 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 |
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 <address@hidden>
>> ---
>> 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
- Re: [Qemu-devel] [PATCH v6 39/42] iotests: Add filter commit test cases,
Max Reitz <=