Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/041 | 44 ++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/041.out | 4 ++--
2 files changed, 46 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
index 8568426311..84bc6d6581 100755
--- a/tests/qemu-iotests/041
+++ b/tests/qemu-iotests/041
@@ -1121,6 +1121,50 @@ class TestOrphanedSource(iotests.QMPTestCase):
target='dest-ro')
self.assert_qmp(result, 'error/class', 'GenericError')
+ def test_failing_permission_in_complete(self):
+ self.assert_no_active_block_jobs()
+
+ # Unshare consistent-read on the target
+ # (The mirror job does not care)
+ result = self.vm.qmp('blockdev-add',
+ driver='blkdebug',
+ node_name='dest-perm',
+ image='dest',
+ unshare_child_perms=['consistent-read'])
+ self.assert_qmp(result, 'return', {})
+
+ result = self.vm.qmp('blockdev-mirror', job_id='job', device='src',
+ sync='full', target='dest',
+ filter_node_name='mirror-filter')
+ self.assert_qmp(result, 'return', {})
+
+ # Require consistent-read on the source
+ # (We can only add this node once the job has started, or it
+ # will complain that it does not want to run on non-root nodes)
+ result = self.vm.qmp('blockdev-add',
+ driver='blkdebug',
+ node_name='src-perm',
+ image='src',
+ take_child_perms=['consistent-read'])
+ self.assert_qmp(result, 'return', {})
+
+ # While completing, mirror will attempt to replace src by
+ # dest, which must fail because src-perm requires
+ # consistent-read but dest-perm does not share it; thus
+ # aborting the job when it is supposed to complete
+ self.complete_and_wait('job',
+ completion_error='Operation not permitted')
+
+ # Assert that all of our nodes are still there (except for the
+ # mirror filter, which should be gone despite the failure)
+ nodes = self.vm.qmp('query-named-block-nodes')['return']
+ nodes = list(map(lambda image: image['node-name'], nodes))
+
+ for expect in ['src', 'src-perm', 'dest', 'dest-perm']:
+ self.assertTrue(expect in nodes, '%s disappeared' % expect)
+ self.assertFalse('mirror-filter' in nodes,
+ 'Mirror filter node did not disappear')
+
if __name__ == '__main__':
iotests.main(supported_fmts=['qcow2', 'qed'],
supported_protocols=['file'])
diff --git a/tests/qemu-iotests/041.out b/tests/qemu-iotests/041.out
index 2c448b4239..f496be9197 100644
--- a/tests/qemu-iotests/041.out
+++ b/tests/qemu-iotests/041.out
@@ -1,5 +1,5 @@
-..........................................................................................
+...........................................................................................
----------------------------------------------------------------------
-Ran 90 tests
+Ran 91 tests
OK
--
2.21.0
12.09.2019 16:56, Max Reitz wrote:
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> tests/qemu-iotests/041 | 44 ++++++++++++++++++++++++++++++++++++++
> tests/qemu-iotests/041.out | 4 ++--
> 2 files changed, 46 insertions(+), 2 deletions(-)
>
> diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
> index 8568426311..84bc6d6581 100755
> --- a/tests/qemu-iotests/041
> +++ b/tests/qemu-iotests/041
> @@ -1121,6 +1121,50 @@ class TestOrphanedSource(iotests.QMPTestCase):
> target='dest-ro')
> self.assert_qmp(result, 'error/class', 'GenericError')
>
> + def test_failing_permission_in_complete(self):
> + self.assert_no_active_block_jobs()
> +
> + # Unshare consistent-read on the target
> + # (The mirror job does not care)
> + result = self.vm.qmp('blockdev-add',
> + driver='blkdebug',
> + node_name='dest-perm',
> + image='dest',
> + unshare_child_perms=['consistent-read'])
> + self.assert_qmp(result, 'return', {})
> +
> + result = self.vm.qmp('blockdev-mirror', job_id='job', device='src',
> + sync='full', target='dest',
> + filter_node_name='mirror-filter')
> + self.assert_qmp(result, 'return', {})
> +
> + # Require consistent-read on the source
> + # (We can only add this node once the job has started, or it
> + # will complain that it does not want to run on non-root nodes)
> + result = self.vm.qmp('blockdev-add',
> + driver='blkdebug',
> + node_name='src-perm',
> + image='src',
> + take_child_perms=['consistent-read'])
> + self.assert_qmp(result, 'return', {})
> +
> + # While completing, mirror will attempt to replace src by
> + # dest, which must fail because src-perm requires
> + # consistent-read but dest-perm does not share it; thus
> + # aborting the job when it is supposed to complete
> + self.complete_and_wait('job',
> + completion_error='Operation not permitted')
> +
> + # Assert that all of our nodes are still there (except for the
> + # mirror filter, which should be gone despite the failure)
> + nodes = self.vm.qmp('query-named-block-nodes')['return']
> + nodes = list(map(lambda image: image['node-name'], nodes))
using list comprehension is a bit more pythonic:
nodes = [node['node-name'] for node in nodes]
> +
> + for expect in ['src', 'src-perm', 'dest', 'dest-perm']:
> + self.assertTrue(expect in nodes, '%s disappeared' % expect)
> + self.assertFalse('mirror-filter' in nodes,
> + 'Mirror filter node did not disappear')
> +
> if __name__ == '__main__':
> iotests.main(supported_fmts=['qcow2', 'qed'],
> supported_protocols=['file'])
> diff --git a/tests/qemu-iotests/041.out b/tests/qemu-iotests/041.out
> index 2c448b4239..f496be9197 100644
> --- a/tests/qemu-iotests/041.out
> +++ b/tests/qemu-iotests/041.out
> @@ -1,5 +1,5 @@
> -..........................................................................................
> +...........................................................................................
> ----------------------------------------------------------------------
> -Ran 90 tests
> +Ran 91 tests
>
> OK
>
With or without my suggestion:
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
I checked, that it pass, and that fails (generates segfault) if drop patch 01:
Tested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
--
Best regards,
Vladimir
On 18.09.19 18:30, Vladimir Sementsov-Ogievskiy wrote:
> 12.09.2019 16:56, Max Reitz wrote:
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>> tests/qemu-iotests/041 | 44 ++++++++++++++++++++++++++++++++++++++
>> tests/qemu-iotests/041.out | 4 ++--
>> 2 files changed, 46 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
>> index 8568426311..84bc6d6581 100755
>> --- a/tests/qemu-iotests/041
>> +++ b/tests/qemu-iotests/041
>> @@ -1121,6 +1121,50 @@ class TestOrphanedSource(iotests.QMPTestCase):
>> target='dest-ro')
>> self.assert_qmp(result, 'error/class', 'GenericError')
>>
>> + def test_failing_permission_in_complete(self):
>> + self.assert_no_active_block_jobs()
>> +
>> + # Unshare consistent-read on the target
>> + # (The mirror job does not care)
>> + result = self.vm.qmp('blockdev-add',
>> + driver='blkdebug',
>> + node_name='dest-perm',
>> + image='dest',
>> + unshare_child_perms=['consistent-read'])
>> + self.assert_qmp(result, 'return', {})
>> +
>> + result = self.vm.qmp('blockdev-mirror', job_id='job', device='src',
>> + sync='full', target='dest',
>> + filter_node_name='mirror-filter')
>> + self.assert_qmp(result, 'return', {})
>> +
>> + # Require consistent-read on the source
>> + # (We can only add this node once the job has started, or it
>> + # will complain that it does not want to run on non-root nodes)
>> + result = self.vm.qmp('blockdev-add',
>> + driver='blkdebug',
>> + node_name='src-perm',
>> + image='src',
>> + take_child_perms=['consistent-read'])
>> + self.assert_qmp(result, 'return', {})
>> +
>> + # While completing, mirror will attempt to replace src by
>> + # dest, which must fail because src-perm requires
>> + # consistent-read but dest-perm does not share it; thus
>> + # aborting the job when it is supposed to complete
>> + self.complete_and_wait('job',
>> + completion_error='Operation not permitted')
>> +
>> + # Assert that all of our nodes are still there (except for the
>> + # mirror filter, which should be gone despite the failure)
>> + nodes = self.vm.qmp('query-named-block-nodes')['return']
>> + nodes = list(map(lambda image: image['node-name'], nodes))
>
> using list comprehension is a bit more pythonic:
> nodes = [node['node-name'] for node in nodes]
OK. I can never remember, so I rarely bother using list/dict
comprehensions and instead use what I would do in any other language.
(Hence the “Sadly” from John.)
(And then wait for some kind reviewer to tell me how to do it right. :-))
Max
On 9/12/19 9:56 AM, Max Reitz wrote:
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> tests/qemu-iotests/041 | 44 ++++++++++++++++++++++++++++++++++++++
> tests/qemu-iotests/041.out | 4 ++--
> 2 files changed, 46 insertions(+), 2 deletions(-)
>
> diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
> index 8568426311..84bc6d6581 100755
> --- a/tests/qemu-iotests/041
> +++ b/tests/qemu-iotests/041
> @@ -1121,6 +1121,50 @@ class TestOrphanedSource(iotests.QMPTestCase):
> target='dest-ro')
> self.assert_qmp(result, 'error/class', 'GenericError')
>
> + def test_failing_permission_in_complete(self):
> + self.assert_no_active_block_jobs()
> +
> + # Unshare consistent-read on the target
> + # (The mirror job does not care)
> + result = self.vm.qmp('blockdev-add',
> + driver='blkdebug',
> + node_name='dest-perm',
> + image='dest',
> + unshare_child_perms=['consistent-read'])
> + self.assert_qmp(result, 'return', {})
> +
> + result = self.vm.qmp('blockdev-mirror', job_id='job', device='src',
> + sync='full', target='dest',
> + filter_node_name='mirror-filter')
> + self.assert_qmp(result, 'return', {})
> +
> + # Require consistent-read on the source
> + # (We can only add this node once the job has started, or it
> + # will complain that it does not want to run on non-root nodes)
> + result = self.vm.qmp('blockdev-add',
> + driver='blkdebug',
> + node_name='src-perm',
> + image='src',
> + take_child_perms=['consistent-read'])
> + self.assert_qmp(result, 'return', {})
> +
> + # While completing, mirror will attempt to replace src by
> + # dest, which must fail because src-perm requires
> + # consistent-read but dest-perm does not share it; thus
> + # aborting the job when it is supposed to complete
> + self.complete_and_wait('job',
> + completion_error='Operation not permitted')
> +
> + # Assert that all of our nodes are still there (except for the
> + # mirror filter, which should be gone despite the failure)
> + nodes = self.vm.qmp('query-named-block-nodes')['return']
> + nodes = list(map(lambda image: image['node-name'], nodes))
Sadly, the list comprehension is a good suggestion. Squash it in if
you'd like.
> +
> + for expect in ['src', 'src-perm', 'dest', 'dest-perm']:
> + self.assertTrue(expect in nodes, '%s disappeared' % expect)
I could be a real weenie and say "why not use a tuple here?"
but, I'll only pretend I didn't say that instead of couching it in a
self-deprecating wrapper to the same end effect.
(I'm a weenie.)
> + self.assertFalse('mirror-filter' in nodes,
> + 'Mirror filter node did not disappear')
> +
> if __name__ == '__main__':
> iotests.main(supported_fmts=['qcow2', 'qed'],
> supported_protocols=['file'])
> diff --git a/tests/qemu-iotests/041.out b/tests/qemu-iotests/041.out
> index 2c448b4239..f496be9197 100644
> --- a/tests/qemu-iotests/041.out
> +++ b/tests/qemu-iotests/041.out
> @@ -1,5 +1,5 @@
> -..........................................................................................
> +...........................................................................................
> ----------------------------------------------------------------------
> -Ran 90 tests
> +Ran 91 tests
>
> OK
>
Or don't do anything, because I'm just being obnoxious, and I owe you
one for giving you bad advice last round.
Reviewed-by: John Snow <jsnow@redhat.com>
On 18.09.19 20:46, John Snow wrote:
>
>
> On 9/12/19 9:56 AM, Max Reitz wrote:
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>> tests/qemu-iotests/041 | 44 ++++++++++++++++++++++++++++++++++++++
>> tests/qemu-iotests/041.out | 4 ++--
>> 2 files changed, 46 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
>> index 8568426311..84bc6d6581 100755
>> --- a/tests/qemu-iotests/041
>> +++ b/tests/qemu-iotests/041
>> @@ -1121,6 +1121,50 @@ class TestOrphanedSource(iotests.QMPTestCase):
>> target='dest-ro')
>> self.assert_qmp(result, 'error/class', 'GenericError')
>> + def test_failing_permission_in_complete(self):
>> + self.assert_no_active_block_jobs()
>> +
>> + # Unshare consistent-read on the target
>> + # (The mirror job does not care)
>> + result = self.vm.qmp('blockdev-add',
>> + driver='blkdebug',
>> + node_name='dest-perm',
>> + image='dest',
>> + unshare_child_perms=['consistent-read'])
>> + self.assert_qmp(result, 'return', {})
>> +
>> + result = self.vm.qmp('blockdev-mirror', job_id='job',
>> device='src',
>> + sync='full', target='dest',
>> + filter_node_name='mirror-filter')
>> + self.assert_qmp(result, 'return', {})
>> +
>> + # Require consistent-read on the source
>> + # (We can only add this node once the job has started, or it
>> + # will complain that it does not want to run on non-root nodes)
>> + result = self.vm.qmp('blockdev-add',
>> + driver='blkdebug',
>> + node_name='src-perm',
>> + image='src',
>> + take_child_perms=['consistent-read'])
>> + self.assert_qmp(result, 'return', {})
>> +
>> + # While completing, mirror will attempt to replace src by
>> + # dest, which must fail because src-perm requires
>> + # consistent-read but dest-perm does not share it; thus
>> + # aborting the job when it is supposed to complete
>> + self.complete_and_wait('job',
>> + completion_error='Operation not
>> permitted')
>> +
>> + # Assert that all of our nodes are still there (except for the
>> + # mirror filter, which should be gone despite the failure)
>> + nodes = self.vm.qmp('query-named-block-nodes')['return']
>> + nodes = list(map(lambda image: image['node-name'], nodes))
>
> Sadly, the list comprehension is a good suggestion. Squash it in if
> you'd like.
You know I don’t, but I’ll do it anyway.
>> +
>> + for expect in ['src', 'src-perm', 'dest', 'dest-perm']:
>> + self.assertTrue(expect in nodes, '%s disappeared' % expect)
>
> I could be a real weenie and say "why not use a tuple here?"
OK.
> but, I'll only pretend I didn't say that instead of couching it in a
> self-deprecating wrapper to the same end effect.
>
> (I'm a weenie.)
>
>> + self.assertFalse('mirror-filter' in nodes,
>> + 'Mirror filter node did not disappear')
>> +
>> if __name__ == '__main__':
>> iotests.main(supported_fmts=['qcow2', 'qed'],
>> supported_protocols=['file'])
>> diff --git a/tests/qemu-iotests/041.out b/tests/qemu-iotests/041.out
>> index 2c448b4239..f496be9197 100644
>> --- a/tests/qemu-iotests/041.out
>> +++ b/tests/qemu-iotests/041.out
>> @@ -1,5 +1,5 @@
>> -..........................................................................................
>>
>> +...........................................................................................
>>
>> ----------------------------------------------------------------------
>> -Ran 90 tests
>> +Ran 91 tests
>> OK
>>
>
> Or don't do anything, because I'm just being obnoxious, and I owe you
> one for giving you bad advice last round.
Come on, I have better (more selfish) reasons.
For the list comprehension: I want to introduce as many instances of
map/filter as I can so using those functions becomes normal.
For the tuple: This is a test, nobody cares whether it uses 60 bytes
more memory or is 10 µs slower or lets something be mutable when it is
actually never changed. As such, I like to use the most general
solution which is simply a list.
Max
On 9/19/19 12:58 PM, Max Reitz wrote:
> On 18.09.19 20:46, John Snow wrote:
>>
>>
>> On 9/12/19 9:56 AM, Max Reitz wrote:
>>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>>> ---
>>> tests/qemu-iotests/041 | 44 ++++++++++++++++++++++++++++++++++++++
>>> tests/qemu-iotests/041.out | 4 ++--
>>> 2 files changed, 46 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
>>> index 8568426311..84bc6d6581 100755
>>> --- a/tests/qemu-iotests/041
>>> +++ b/tests/qemu-iotests/041
>>> @@ -1121,6 +1121,50 @@ class TestOrphanedSource(iotests.QMPTestCase):
>>> target='dest-ro')
>>> self.assert_qmp(result, 'error/class', 'GenericError')
>>> + def test_failing_permission_in_complete(self):
>>> + self.assert_no_active_block_jobs()
>>> +
>>> + # Unshare consistent-read on the target
>>> + # (The mirror job does not care)
>>> + result = self.vm.qmp('blockdev-add',
>>> + driver='blkdebug',
>>> + node_name='dest-perm',
>>> + image='dest',
>>> + unshare_child_perms=['consistent-read'])
>>> + self.assert_qmp(result, 'return', {})
>>> +
>>> + result = self.vm.qmp('blockdev-mirror', job_id='job',
>>> device='src',
>>> + sync='full', target='dest',
>>> + filter_node_name='mirror-filter')
>>> + self.assert_qmp(result, 'return', {})
>>> +
>>> + # Require consistent-read on the source
>>> + # (We can only add this node once the job has started, or it
>>> + # will complain that it does not want to run on non-root nodes)
>>> + result = self.vm.qmp('blockdev-add',
>>> + driver='blkdebug',
>>> + node_name='src-perm',
>>> + image='src',
>>> + take_child_perms=['consistent-read'])
>>> + self.assert_qmp(result, 'return', {})
>>> +
>>> + # While completing, mirror will attempt to replace src by
>>> + # dest, which must fail because src-perm requires
>>> + # consistent-read but dest-perm does not share it; thus
>>> + # aborting the job when it is supposed to complete
>>> + self.complete_and_wait('job',
>>> + completion_error='Operation not
>>> permitted')
>>> +
>>> + # Assert that all of our nodes are still there (except for the
>>> + # mirror filter, which should be gone despite the failure)
>>> + nodes = self.vm.qmp('query-named-block-nodes')['return']
>>> + nodes = list(map(lambda image: image['node-name'], nodes))
>>
>> Sadly, the list comprehension is a good suggestion. Squash it in if
>> you'd like.
>
> You know I don’t, but I’ll do it anyway.
>
Don't you dare make me feel bad by re-spinning, though.
>>> +
>>> + for expect in ['src', 'src-perm', 'dest', 'dest-perm']:
>>> + self.assertTrue(expect in nodes, '%s disappeared' % expect)
>>
>> I could be a real weenie and say "why not use a tuple here?"
>
> OK.
>
>> but, I'll only pretend I didn't say that instead of couching it in a
>> self-deprecating wrapper to the same end effect.
>>
>> (I'm a weenie.)
>>
>>> + self.assertFalse('mirror-filter' in nodes,
>>> + 'Mirror filter node did not disappear')
>>> +
>>> if __name__ == '__main__':
>>> iotests.main(supported_fmts=['qcow2', 'qed'],
>>> supported_protocols=['file'])
>>> diff --git a/tests/qemu-iotests/041.out b/tests/qemu-iotests/041.out
>>> index 2c448b4239..f496be9197 100644
>>> --- a/tests/qemu-iotests/041.out
>>> +++ b/tests/qemu-iotests/041.out
>>> @@ -1,5 +1,5 @@
>>> -..........................................................................................
>>>
>>> +...........................................................................................
>>>
>>> ----------------------------------------------------------------------
>>> -Ran 90 tests
>>> +Ran 91 tests
>>> OK
>>>
>>
>> Or don't do anything, because I'm just being obnoxious, and I owe you
>> one for giving you bad advice last round.
>
> Come on, I have better (more selfish) reasons.
>
> For the list comprehension: I want to introduce as many instances of
> map/filter as I can so using those functions becomes normal.
>
They have their uses! But also they're usually just simply longer and
aren't worth using where list comprehensions do.
> For the tuple: This is a test, nobody cares whether it uses 60 bytes
> more memory or is 10 µs slower or lets something be mutable when it is
> actually never changed. As such, I like to use the most general
> solution which is simply a list.
>
I did say I was being a weenie. You really can take the reviewed-by!
On 19.09.19 19:02, John Snow wrote:
>
>
> On 9/19/19 12:58 PM, Max Reitz wrote:
>> On 18.09.19 20:46, John Snow wrote:
>>>
>>>
>>> On 9/12/19 9:56 AM, Max Reitz wrote:
>>>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>>>> ---
>>>> tests/qemu-iotests/041 | 44 ++++++++++++++++++++++++++++++++++++++
>>>> tests/qemu-iotests/041.out | 4 ++--
>>>> 2 files changed, 46 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
>>>> index 8568426311..84bc6d6581 100755
>>>> --- a/tests/qemu-iotests/041
>>>> +++ b/tests/qemu-iotests/041
>>>> @@ -1121,6 +1121,50 @@ class TestOrphanedSource(iotests.QMPTestCase):
>>>> target='dest-ro')
>>>> self.assert_qmp(result, 'error/class', 'GenericError')
>>>> + def test_failing_permission_in_complete(self):
>>>> + self.assert_no_active_block_jobs()
>>>> +
>>>> + # Unshare consistent-read on the target
>>>> + # (The mirror job does not care)
>>>> + result = self.vm.qmp('blockdev-add',
>>>> + driver='blkdebug',
>>>> + node_name='dest-perm',
>>>> + image='dest',
>>>> + unshare_child_perms=['consistent-read'])
>>>> + self.assert_qmp(result, 'return', {})
>>>> +
>>>> + result = self.vm.qmp('blockdev-mirror', job_id='job',
>>>> device='src',
>>>> + sync='full', target='dest',
>>>> + filter_node_name='mirror-filter')
>>>> + self.assert_qmp(result, 'return', {})
>>>> +
>>>> + # Require consistent-read on the source
>>>> + # (We can only add this node once the job has started, or it
>>>> + # will complain that it does not want to run on non-root nodes)
>>>> + result = self.vm.qmp('blockdev-add',
>>>> + driver='blkdebug',
>>>> + node_name='src-perm',
>>>> + image='src',
>>>> + take_child_perms=['consistent-read'])
>>>> + self.assert_qmp(result, 'return', {})
>>>> +
>>>> + # While completing, mirror will attempt to replace src by
>>>> + # dest, which must fail because src-perm requires
>>>> + # consistent-read but dest-perm does not share it; thus
>>>> + # aborting the job when it is supposed to complete
>>>> + self.complete_and_wait('job',
>>>> + completion_error='Operation not
>>>> permitted')
>>>> +
>>>> + # Assert that all of our nodes are still there (except for the
>>>> + # mirror filter, which should be gone despite the failure)
>>>> + nodes = self.vm.qmp('query-named-block-nodes')['return']
>>>> + nodes = list(map(lambda image: image['node-name'], nodes))
>>>
>>> Sadly, the list comprehension is a good suggestion. Squash it in if
>>> you'd like.
>>
>> You know I don’t, but I’ll do it anyway.
>>
>
> Don't you dare make me feel bad by re-spinning, though.
I have to respin anyway. ;-)
>>>> +
>>>> + for expect in ['src', 'src-perm', 'dest', 'dest-perm']:
>>>> + self.assertTrue(expect in nodes, '%s disappeared' % expect)
>>>
>>> I could be a real weenie and say "why not use a tuple here?"
>>
>> OK.
>>
>>> but, I'll only pretend I didn't say that instead of couching it in a
>>> self-deprecating wrapper to the same end effect.
>>>
>>> (I'm a weenie.)
>>>
>>>> + self.assertFalse('mirror-filter' in nodes,
>>>> + 'Mirror filter node did not disappear')
>>>> +
>>>> if __name__ == '__main__':
>>>> iotests.main(supported_fmts=['qcow2', 'qed'],
>>>> supported_protocols=['file'])
>>>> diff --git a/tests/qemu-iotests/041.out b/tests/qemu-iotests/041.out
>>>> index 2c448b4239..f496be9197 100644
>>>> --- a/tests/qemu-iotests/041.out
>>>> +++ b/tests/qemu-iotests/041.out
>>>> @@ -1,5 +1,5 @@
>>>> -..........................................................................................
>>>>
>>>> +...........................................................................................
>>>>
>>>> ----------------------------------------------------------------------
>>>> -Ran 90 tests
>>>> +Ran 91 tests
>>>> OK
>>>>
>>>
>>> Or don't do anything, because I'm just being obnoxious, and I owe you
>>> one for giving you bad advice last round.
>>
>> Come on, I have better (more selfish) reasons.
>>
>> For the list comprehension: I want to introduce as many instances of
>> map/filter as I can so using those functions becomes normal.
>>
>
> They have their uses! But also they're usually just simply longer and
> aren't worth using where list comprehensions do.
The point is that they’re special language constructs whereas map/filter
are available in basically any decent language.
>> For the tuple: This is a test, nobody cares whether it uses 60 bytes
>> more memory or is 10 µs slower or lets something be mutable when it is
>> actually never changed. As such, I like to use the most general
>> solution which is simply a list.
>>
>
> I did say I was being a weenie. You really can take the reviewed-by!
I’m just saying those would be my reasons if I rejected the changes
suggested by you both. I don’t reject them, though. :-)
Max
© 2016 - 2025 Red Hat, Inc.