Re: [Qemu-block] [PATCH v6 39/42] iotests: Add filter commit test cases

2019-09-02 Thread Max Reitz
On 31.08.19 14:35, 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 
>> ---
>>   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
>> @@ -428,5 +428,182 @@ class TestReopenOverlay(ImageCommitTestCase):
>>   def test_reopen_overlay(self):
>>   self.run_commit_test(self.img1, self.img0)
>>   
>> +class TestCommitWithFilters(iotests.QMPTestCase):
>> +img0 = os.path.join(iotests.test_dir, '0.img')
>> +img1 = os.path.join(iotests.test_dir, '1.img')
>> +img2 = os.path.join(iotests.test_dir, '2.img')
>> +img3 = os.path.join(iotests.test_dir, '3.img')
>> +
>> +def setUp(self):
>> +qemu_img('create', '-f', iotests.imgfmt, self.img0, '64M')
>> +qemu_img('create', '-f', iotests.imgfmt, self.img1, '64M')
>> +qemu_img('create', '-f', iotests.imgfmt, self.img2, '64M')
>> +qemu_img('create', '-f', iotests.imgfmt, self.img3, '64M')
>> +
>> +qemu_io('-f', iotests.imgfmt, '-c', 'write -P 1 0M 1M', self.img0)
>> +qemu_io('-f', iotests.imgfmt, '-c', 'write -P 2 1M 1M', self.img1)
>> +qemu_io('-f', iotests.imgfmt, '-c', 'write -P 3 2M 1M', self.img2)
>> +qemu_io('-f', iotests.imgfmt, '-c', 'write -P 4 3M 1M', self.img3)
>> +
>> +# Distributions of the patterns in the files; this is checked
>> +# by tearDown() and should be changed by the test cases as is
>> +# necessary
>> +self.pattern_files = [self.img0, self.img1, self.img2, self.img3]
>> +
>> +self.vm = iotests.VM()
>> +self.vm.launch()
>> +self.has_quit = False
> 
> has_quit is unused actually. It's always False.

True. (:-))  I wonder why I added it.

Max



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-block] [PATCH v6 39/42] iotests: Add filter commit test cases

2019-09-02 Thread Max Reitz
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 
>> ---
>>   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-block] [PATCH v6 39/42] iotests: Add filter commit test cases

2019-08-31 Thread Vladimir Sementsov-Ogievskiy
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 
> ---
>   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
> @@ -428,5 +428,182 @@ class TestReopenOverlay(ImageCommitTestCase):
>   def test_reopen_overlay(self):
>   self.run_commit_test(self.img1, self.img0)
>   
> +class TestCommitWithFilters(iotests.QMPTestCase):
> +img0 = os.path.join(iotests.test_dir, '0.img')
> +img1 = os.path.join(iotests.test_dir, '1.img')
> +img2 = os.path.join(iotests.test_dir, '2.img')
> +img3 = os.path.join(iotests.test_dir, '3.img')
> +
> +def setUp(self):
> +qemu_img('create', '-f', iotests.imgfmt, self.img0, '64M')
> +qemu_img('create', '-f', iotests.imgfmt, self.img1, '64M')
> +qemu_img('create', '-f', iotests.imgfmt, self.img2, '64M')
> +qemu_img('create', '-f', iotests.imgfmt, self.img3, '64M')
> +
> +qemu_io('-f', iotests.imgfmt, '-c', 'write -P 1 0M 1M', self.img0)
> +qemu_io('-f', iotests.imgfmt, '-c', 'write -P 2 1M 1M', self.img1)
> +qemu_io('-f', iotests.imgfmt, '-c', 'write -P 3 2M 1M', self.img2)
> +qemu_io('-f', iotests.imgfmt, '-c', 'write -P 4 3M 1M', self.img3)
> +
> +# Distributions of the patterns in the files; this is checked
> +# by tearDown() and should be changed by the test cases as is
> +# necessary
> +self.pattern_files = [self.img0, self.img1, self.img2, self.img3]
> +
> +self.vm = iotests.VM()
> +self.vm.launch()
> +self.has_quit = False

has_quit is unused actually. It's always False.



-- 
Best regards,
Vladimir


Re: [Qemu-block] [PATCH v6 39/42] iotests: Add filter commit test cases

2019-08-31 Thread Vladimir Sementsov-Ogievskiy
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 
> ---
>   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
> @@ -428,5 +428,182 @@ class TestReopenOverlay(ImageCommitTestCase):
>   def test_reopen_overlay(self):
>   self.run_commit_test(self.img1, self.img0)
>   
> +class TestCommitWithFilters(iotests.QMPTestCase):
> +img0 = os.path.join(iotests.test_dir, '0.img')
> +img1 = os.path.join(iotests.test_dir, '1.img')
> +img2 = os.path.join(iotests.test_dir, '2.img')
> +img3 = os.path.join(iotests.test_dir, '3.img')
> +
> +def setUp(self):
> +qemu_img('create', '-f', iotests.imgfmt, self.img0, '64M')
> +qemu_img('create', '-f', iotests.imgfmt, self.img1, '64M')
> +qemu_img('create', '-f', iotests.imgfmt, self.img2, '64M')
> +qemu_img('create', '-f', iotests.imgfmt, self.img3, '64M')
> +
> +qemu_io('-f', iotests.imgfmt, '-c', 'write -P 1 0M 1M', self.img0)
> +qemu_io('-f', iotests.imgfmt, '-c', 'write -P 2 1M 1M', self.img1)
> +qemu_io('-f', iotests.imgfmt, '-c', 'write -P 3 2M 1M', self.img2)
> +qemu_io('-f', iotests.imgfmt, '-c', 'write -P 4 3M 1M', self.img3)
> +
> +# Distributions of the patterns in the files; this is checked
> +# by tearDown() and should be changed by the test cases as is
> +# necessary
> +self.pattern_files = [self.img0, self.img1, self.img2, self.img3]
> +
> +self.vm = iotests.VM()
> +self.vm.launch()
> +self.has_quit = False
> +
> +result = self.vm.qmp('object-add', qom_type='throttle-group', 
> id='tg')
> +self.assert_qmp(result, 'return', {})
> +
> +result = self.vm.qmp('blockdev-add', **{
> +'node-name': 'top-filter',
> +'driver': 'throttle',
> +'throttle-group': 'tg',
> +'file': {
> +'node-name': 'cow-3',
> +'driver': iotests.imgfmt,
> +'file': {
> +'driver': 'file',
> +'filename': self.img3
> +},
> +'backing': {
> +'node-name': 'cow-2',
> +'driver': iotests.imgfmt,
> +'file': {
> +'driver': 'file',
> +'filename': self.img2
> +},
> +'backing': {
> +'node-name': 'cow-1',
> +'driver': iotests.imgfmt,
> +'file': {
> +'driver': 'file',
> +'filename': self.img1
> +},
> +'backing': {
> +'node-name': 'bottom-filter',
> +'driver': 'throttle',
> +'throttle-group': 'tg',
> +'file': {
> +'node-name': 'cow-0',
> +'driver': iotests.imgfmt,
> +'file': {
> +'driver': 'file',
> +'filename': self.img0
> +}
> +}
> +}
> +}
> +}
> +}
> +})
> +self.assert_qmp(result, 'return', {})
> +
> +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):
...

> +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.

> +
> +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

[Qemu-block] [PATCH v6 39/42] iotests: Add filter commit test cases

2019-08-09 Thread Max Reitz
This patch adds some tests on how commit copes with filter nodes.

Signed-off-by: Max Reitz 
---
 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
@@ -428,5 +428,182 @@ class TestReopenOverlay(ImageCommitTestCase):
 def test_reopen_overlay(self):
 self.run_commit_test(self.img1, self.img0)
 
+class TestCommitWithFilters(iotests.QMPTestCase):
+img0 = os.path.join(iotests.test_dir, '0.img')
+img1 = os.path.join(iotests.test_dir, '1.img')
+img2 = os.path.join(iotests.test_dir, '2.img')
+img3 = os.path.join(iotests.test_dir, '3.img')
+
+def setUp(self):
+qemu_img('create', '-f', iotests.imgfmt, self.img0, '64M')
+qemu_img('create', '-f', iotests.imgfmt, self.img1, '64M')
+qemu_img('create', '-f', iotests.imgfmt, self.img2, '64M')
+qemu_img('create', '-f', iotests.imgfmt, self.img3, '64M')
+
+qemu_io('-f', iotests.imgfmt, '-c', 'write -P 1 0M 1M', self.img0)
+qemu_io('-f', iotests.imgfmt, '-c', 'write -P 2 1M 1M', self.img1)
+qemu_io('-f', iotests.imgfmt, '-c', 'write -P 3 2M 1M', self.img2)
+qemu_io('-f', iotests.imgfmt, '-c', 'write -P 4 3M 1M', self.img3)
+
+# Distributions of the patterns in the files; this is checked
+# by tearDown() and should be changed by the test cases as is
+# necessary
+self.pattern_files = [self.img0, self.img1, self.img2, self.img3]
+
+self.vm = iotests.VM()
+self.vm.launch()
+self.has_quit = False
+
+result = self.vm.qmp('object-add', qom_type='throttle-group', id='tg')
+self.assert_qmp(result, 'return', {})
+
+result = self.vm.qmp('blockdev-add', **{
+'node-name': 'top-filter',
+'driver': 'throttle',
+'throttle-group': 'tg',
+'file': {
+'node-name': 'cow-3',
+'driver': iotests.imgfmt,
+'file': {
+'driver': 'file',
+'filename': self.img3
+},
+'backing': {
+'node-name': 'cow-2',
+'driver': iotests.imgfmt,
+'file': {
+'driver': 'file',
+'filename': self.img2
+},
+'backing': {
+'node-name': 'cow-1',
+'driver': iotests.imgfmt,
+'file': {
+'driver': 'file',
+'filename': self.img1
+},
+'backing': {
+'node-name': 'bottom-filter',
+'driver': 'throttle',
+'throttle-group': 'tg',
+'file': {
+'node-name': 'cow-0',
+'driver': iotests.imgfmt,
+'file': {
+'driver': 'file',
+'filename': self.img0
+}
+}
+}
+}
+}
+}
+})
+self.assert_qmp(result, 'return', {})
+
+def tearDown(self):
+self.vm.shutdown(has_quit=self.has_quit)
+
+for index in range(len(self.pattern_files)):
+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)
+
+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']
+
+def test_filterless_commit(self):
+self.assert_no_active_block_jobs()
+result = self.vm.qmp('block-commit',
+ job_id='commit',
+ device='top-filter',
+ top_node='cow-2',
+ base_node='cow-1')
+self.assert_qmp(result, 'return', {})
+self.wait_until_completed(drive='commit')
+
+self.assertIsNotNone(self.vm.node_info('cow-3'))
+self.assertIsNone(self.vm.node_info('cow-2'))
+self.assertIsNo