From: Peter Krempa <pkrempa@redhat.com>
Verify that the modification of the bitmap persistence over migration
which is controlled via BitmapMigrationBitmapAliasTransform works
properly.
Based on TestCrossAliasMigration
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Message-Id: <d9c8e9827e9b6001b2dd1b92e64aab858e6d2a86.1613150869.git.pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[eblake: Adjust test for explicit read_zeroes=False]
Signed-off-by: Eric Blake <eblake@redhat.com>
---
tests/qemu-iotests/300 | 93 ++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/300.out | 4 +-
2 files changed, 95 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/300 b/tests/qemu-iotests/300
index 43264d883d79..63036f6a6e13 100755
--- a/tests/qemu-iotests/300
+++ b/tests/qemu-iotests/300
@@ -600,6 +600,99 @@ class TestCrossAliasMigration(TestDirtyBitmapMigration):
self.verify_dest_has_all_bitmaps()
self.verify_dest_error(None)
+class TestAliasTransformMigration(TestDirtyBitmapMigration):
+ """
+ Tests the 'transform' option which modifies bitmap persistence on migration.
+ """
+
+ src_node_name = 'node-a'
+ dst_node_name = 'node-b'
+ src_bmap_name = 'bmap-a'
+ dst_bmap_name = 'bmap-b'
+
+ def setUp(self) -> None:
+ TestDirtyBitmapMigration.setUp(self)
+
+ # Now create another block device and let both have two bitmaps each
+ result = self.vm_a.qmp('blockdev-add',
+ node_name='node-b', driver='null-co',
+ read_zeroes=False)
+ self.assert_qmp(result, 'return', {})
+
+ result = self.vm_b.qmp('blockdev-add',
+ node_name='node-a', driver='null-co',
+ read_zeroes=False)
+ self.assert_qmp(result, 'return', {})
+
+ bmaps_to_add = (('node-a', 'bmap-b'),
+ ('node-b', 'bmap-a'),
+ ('node-b', 'bmap-b'))
+
+ for (node, bmap) in bmaps_to_add:
+ result = self.vm_a.qmp('block-dirty-bitmap-add',
+ node=node, name=bmap)
+ self.assert_qmp(result, 'return', {})
+
+ @staticmethod
+ def transform_mapping() -> BlockBitmapMapping:
+ return [
+ {
+ 'node-name': 'node-a',
+ 'alias': 'node-a',
+ 'bitmaps': [
+ {
+ 'name': 'bmap-a',
+ 'alias': 'bmap-a',
+ 'transform':
+ {
+ 'persistent': True
+ }
+ },
+ {
+ 'name': 'bmap-b',
+ 'alias': 'bmap-b'
+ }
+ ]
+ },
+ {
+ 'node-name': 'node-b',
+ 'alias': 'node-b',
+ 'bitmaps': [
+ {
+ 'name': 'bmap-a',
+ 'alias': 'bmap-a'
+ },
+ {
+ 'name': 'bmap-b',
+ 'alias': 'bmap-b'
+ }
+ ]
+ }
+ ]
+
+ def verify_dest_bitmap_state(self) -> None:
+ bitmaps = self.vm_b.query_bitmaps()
+
+ for node in bitmaps:
+ bitmaps[node] = sorted(((bmap['name'], bmap['persistent']) for bmap in bitmaps[node]))
+
+ self.assertEqual(bitmaps,
+ {'node-a': [('bmap-a', True), ('bmap-b', False)],
+ 'node-b': [('bmap-a', False), ('bmap-b', False)]})
+
+ def test_transform_on_src(self) -> None:
+ self.set_mapping(self.vm_a, self.transform_mapping())
+
+ self.migrate()
+ self.verify_dest_bitmap_state()
+ self.verify_dest_error(None)
+
+ def test_transform_on_dst(self) -> None:
+ self.set_mapping(self.vm_b, self.transform_mapping())
+
+ self.migrate()
+ self.verify_dest_bitmap_state()
+ self.verify_dest_error(None)
if __name__ == '__main__':
iotests.main(supported_protocols=['file'])
diff --git a/tests/qemu-iotests/300.out b/tests/qemu-iotests/300.out
index cafb8161f7b1..12e9ab7d571b 100644
--- a/tests/qemu-iotests/300.out
+++ b/tests/qemu-iotests/300.out
@@ -1,5 +1,5 @@
-.....................................
+.......................................
----------------------------------------------------------------------
-Ran 37 tests
+Ran 39 tests
OK
--
2.30.1
Am 13.02.2021 um 00:21 hat Eric Blake geschrieben: > From: Peter Krempa <pkrempa@redhat.com> > > Verify that the modification of the bitmap persistence over migration > which is controlled via BitmapMigrationBitmapAliasTransform works > properly. > > Based on TestCrossAliasMigration > > Signed-off-by: Peter Krempa <pkrempa@redhat.com> > Message-Id: <d9c8e9827e9b6001b2dd1b92e64aab858e6d2a86.1613150869.git.pkrempa@redhat.com> > Reviewed-by: Eric Blake <eblake@redhat.com> > [eblake: Adjust test for explicit read_zeroes=False] > Signed-off-by: Eric Blake <eblake@redhat.com> This breaks 297: --- /home/kwolf/source/qemu/tests/qemu-iotests/297.out +++ 297.out.bad @@ -1,2 +1,8 @@ === pylint === +************* Module 300 +300:605:0: C0301: Line too long (80/79) (line-too-long) +300:677:0: C0301: Line too long (98/79) (line-too-long) === mypy === +300:646: error: Dict entry 2 has incompatible type "str": "Dict[str, bool]"; expected "str": "str" +Found 1 error in 1 file (checked 1 source file) +
On 2/15/21 6:31 AM, Kevin Wolf wrote:
> Am 13.02.2021 um 00:21 hat Eric Blake geschrieben:
>> From: Peter Krempa <pkrempa@redhat.com>
>>
>> Verify that the modification of the bitmap persistence over migration
>> which is controlled via BitmapMigrationBitmapAliasTransform works
>> properly.
>>
>> Based on TestCrossAliasMigration
>>
>> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>> Message-Id: <d9c8e9827e9b6001b2dd1b92e64aab858e6d2a86.1613150869.git.pkrempa@redhat.com>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>> [eblake: Adjust test for explicit read_zeroes=False]
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>
> This breaks 297:
>
> --- /home/kwolf/source/qemu/tests/qemu-iotests/297.out
> +++ 297.out.bad
> @@ -1,2 +1,8 @@
> === pylint ===
> +************* Module 300
> +300:605:0: C0301: Line too long (80/79) (line-too-long)
> +300:677:0: C0301: Line too long (98/79) (line-too-long)
These two are easy fixes (add line breaks for shorter lines), but this:
> === mypy ===
> +300:646: error: Dict entry 2 has incompatible type "str": "Dict[str, bool]"; expected "str": "str"
> +Found 1 error in 1 file (checked 1 source file)
is beyond my skill. The typing at line 33:
BlockBitmapMapping = List[Dict[str, Union[str, List[Dict[str, str]]]]]
is insufficient to allow our new 'transform' member in the new
transform_mapping() -> Block BitmapMapping near line 677:
'bitmaps': [
{
'name': 'bmap-a',
'alias': 'bmap-a',
'transform':
{
'persistent': True
}
},
but I'm not sure how to tell python the right type it should be. John?
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Am 15.02.2021 um 17:46 hat Eric Blake geschrieben:
> On 2/15/21 6:31 AM, Kevin Wolf wrote:
> > Am 13.02.2021 um 00:21 hat Eric Blake geschrieben:
> >> From: Peter Krempa <pkrempa@redhat.com>
> >>
> >> Verify that the modification of the bitmap persistence over migration
> >> which is controlled via BitmapMigrationBitmapAliasTransform works
> >> properly.
> >>
> >> Based on TestCrossAliasMigration
> >>
> >> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> >> Message-Id: <d9c8e9827e9b6001b2dd1b92e64aab858e6d2a86.1613150869.git.pkrempa@redhat.com>
> >> Reviewed-by: Eric Blake <eblake@redhat.com>
> >> [eblake: Adjust test for explicit read_zeroes=False]
> >> Signed-off-by: Eric Blake <eblake@redhat.com>
> >
> > This breaks 297:
> >
> > --- /home/kwolf/source/qemu/tests/qemu-iotests/297.out
> > +++ 297.out.bad
> > @@ -1,2 +1,8 @@
> > === pylint ===
> > +************* Module 300
> > +300:605:0: C0301: Line too long (80/79) (line-too-long)
> > +300:677:0: C0301: Line too long (98/79) (line-too-long)
>
> These two are easy fixes (add line breaks for shorter lines), but this:
>
> > === mypy ===
> > +300:646: error: Dict entry 2 has incompatible type "str": "Dict[str, bool]"; expected "str": "str"
> > +Found 1 error in 1 file (checked 1 source file)
>
> is beyond my skill. The typing at line 33:
>
> BlockBitmapMapping = List[Dict[str, Union[str, List[Dict[str, str]]]]]
>
> is insufficient to allow our new 'transform' member in the new
> transform_mapping() -> Block BitmapMapping near line 677:
>
> 'bitmaps': [
> {
> 'name': 'bmap-a',
> 'alias': 'bmap-a',
> 'transform':
> {
> 'persistent': True
> }
> },
>
> but I'm not sure how to tell python the right type it should be. John?
To be honest, this looks sufficiently like JSON that I would just go for
List[Dict[str, Any]] (as long as recursive types don't work), but if you
really want to have an explicit type, I think you'd have to replace the
rightmost str with Union[str, Dict[str, bool]] to allow both.
Kevin
On 2/15/21 11:09 AM, Kevin Wolf wrote:
>>> This breaks 297:
>>> === mypy ===
>>> +300:646: error: Dict entry 2 has incompatible type "str": "Dict[str, bool]"; expected "str": "str"
>>> +Found 1 error in 1 file (checked 1 source file)
>>
>> is beyond my skill. The typing at line 33:
>>
>> BlockBitmapMapping = List[Dict[str, Union[str, List[Dict[str, str]]]]]
>>
>> is insufficient to allow our new 'transform' member in the new
>> transform_mapping() -> Block BitmapMapping near line 677:
>>
>> 'bitmaps': [
>> {
>> 'name': 'bmap-a',
>> 'alias': 'bmap-a',
>> 'transform':
>> {
>> 'persistent': True
>> }
>> },
>>
>> but I'm not sure how to tell python the right type it should be. John?
>
> To be honest, this looks sufficiently like JSON that I would just go for
> List[Dict[str, Any]] (as long as recursive types don't work), but if you
> really want to have an explicit type, I think you'd have to replace the
> rightmost str with Union[str, Dict[str, bool]] to allow both.
Indeed, I played with it before reading your response, and came up with
this. Shall I turn it into a formal patch?
diff --git i/tests/qemu-iotests/300 w/tests/qemu-iotests/300
index 63036f6a6e13..7501bd1018e2 100755
--- i/tests/qemu-iotests/300
+++ w/tests/qemu-iotests/300
@@ -30,7 +30,10 @@ import iotests
# pylint: disable=wrong-import-order
import qemu
-BlockBitmapMapping = List[Dict[str, Union[str, List[Dict[str, str]]]]]
+BlockBitmapMapping = List[Dict[str,
+ Union[str,
+ List[Dict[str,
+ Union[str, Dict[str,
bool]]]]]]]
mig_sock = os.path.join(iotests.sock_dir, 'mig_sock')
@@ -602,7 +605,8 @@ class TestCrossAliasMigration(TestDirtyBitmapMigration):
class TestAliasTransformMigration(TestDirtyBitmapMigration):
"""
- Tests the 'transform' option which modifies bitmap persistence on
migration.
+ Tests the 'transform' option which modifies bitmap persistence on
+ migration.
"""
src_node_name = 'node-a'
@@ -674,7 +678,8 @@ class
TestAliasTransformMigration(TestDirtyBitmapMigration):
bitmaps = self.vm_b.query_bitmaps()
for node in bitmaps:
- bitmaps[node] = sorted(((bmap['name'], bmap['persistent'])
for bmap in bitmaps[node]))
+ bitmaps[node] = sorted(((bmap['name'], bmap['persistent'])
+ for bmap in bitmaps[node]))
self.assertEqual(bitmaps,
{'node-a': [('bmap-a', True), ('bmap-b', False)],
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
On 2/15/21 1:25 PM, Eric Blake wrote: > -BlockBitmapMapping = List[Dict[str, Union[str, List[Dict[str, str]]]]] > +BlockBitmapMapping = List[Dict[str, > + Union[str, > + List[Dict[str, > + Union[str, Dict[str, > bool]]]]]]] That looks *very* beefy. Is the Union because that union is valid for every key, or because every key has a potentially different value that is specific to that key? if it's the latter, I'd ditch the Union and just go with: Dict[str, object], or Dict[str, Any] object: will allow any type, but keeps strict checking enabled. If you try to use that value later on without a cast, mypy will warn you if you are using it in a manner not guaranteed by the "object" type. Can be useful if you are passing values to a function that already does RTTI to determine behavior. Any: Also allows any type, but enables gradual typing. If you later "assume" the type of this value, mypy will say nothing. Can be useful when you've just got a job to do and the right tool would have been a recursive type or a TypedDict (unavailable in Python 3.6.) --js
On 2/15/21 1:00 PM, John Snow wrote: > On 2/15/21 1:25 PM, Eric Blake wrote: >> -BlockBitmapMapping = List[Dict[str, Union[str, List[Dict[str, str]]]]] >> +BlockBitmapMapping = List[Dict[str, >> + Union[str, >> + List[Dict[str, >> + Union[str, Dict[str, >> bool]]]]]]] > > That looks *very* beefy. > > Is the Union because that union is valid for every key, or because every > key has a potentially different value that is specific to that key? > > if it's the latter, I'd ditch the Union and just go with: > > Dict[str, object], or > Dict[str, Any] > > object: will allow any type, but keeps strict checking enabled. If you > try to use that value later on without a cast, mypy will warn you if you > are using it in a manner not guaranteed by the "object" type. Can be > useful if you are passing values to a function that already does RTTI to > determine behavior. We're in luck; both 297 and 300 still pass with this applied on top of my previous attempt: diff --git i/tests/qemu-iotests/300 w/tests/qemu-iotests/300 index 7501bd1018e2..adb927629747 100755 --- i/tests/qemu-iotests/300 +++ w/tests/qemu-iotests/300 @@ -22,7 +22,7 @@ import os import random import re -from typing import Dict, List, Optional, Union +from typing import Dict, List, Optional import iotests @@ -30,10 +30,7 @@ import iotests # pylint: disable=wrong-import-order import qemu -BlockBitmapMapping = List[Dict[str, - Union[str, - List[Dict[str, - Union[str, Dict[str, bool]]]]]]] +BlockBitmapMapping = List[Dict[str, object]] mig_sock = os.path.join(iotests.sock_dir, 'mig_sock') > > Any: Also allows any type, but enables gradual typing. If you later > "assume" the type of this value, mypy will say nothing. Can be useful > when you've just got a job to do and the right tool would have been a > recursive type or a TypedDict (unavailable in Python 3.6.) I'm not too worried about needing to further enhance the type-checking on an individual iotest. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
On 2/15/21 3:26 PM, Eric Blake wrote: > On 2/15/21 1:00 PM, John Snow wrote: >> On 2/15/21 1:25 PM, Eric Blake wrote: >>> -BlockBitmapMapping = List[Dict[str, Union[str, List[Dict[str, str]]]]] >>> +BlockBitmapMapping = List[Dict[str, >>> + Union[str, >>> + List[Dict[str, >>> + Union[str, Dict[str, >>> bool]]]]]]] >> >> That looks *very* beefy. >> >> Is the Union because that union is valid for every key, or because every >> key has a potentially different value that is specific to that key? >> >> if it's the latter, I'd ditch the Union and just go with: >> >> Dict[str, object], or >> Dict[str, Any] >> >> object: will allow any type, but keeps strict checking enabled. If you >> try to use that value later on without a cast, mypy will warn you if you >> are using it in a manner not guaranteed by the "object" type. Can be >> useful if you are passing values to a function that already does RTTI to >> determine behavior. > > We're in luck; both 297 and 300 still pass with this applied on top of > my previous attempt: > > diff --git i/tests/qemu-iotests/300 w/tests/qemu-iotests/300 > index 7501bd1018e2..adb927629747 100755 > --- i/tests/qemu-iotests/300 > +++ w/tests/qemu-iotests/300 > @@ -22,7 +22,7 @@ > import os > import random > import re > -from typing import Dict, List, Optional, Union > +from typing import Dict, List, Optional > > import iotests > > @@ -30,10 +30,7 @@ import iotests > # pylint: disable=wrong-import-order > import qemu > > -BlockBitmapMapping = List[Dict[str, > - Union[str, > - List[Dict[str, > - Union[str, Dict[str, > bool]]]]]]] > +BlockBitmapMapping = List[Dict[str, object]] > nice :) > mig_sock = os.path.join(iotests.sock_dir, 'mig_sock') > > > >> >> Any: Also allows any type, but enables gradual typing. If you later >> "assume" the type of this value, mypy will say nothing. Can be useful >> when you've just got a job to do and the right tool would have been a >> recursive type or a TypedDict (unavailable in Python 3.6.) > > I'm not too worried about needing to further enhance the type-checking > on an individual iotest. > Yes, Agreed. I have been going very "overboard" with the python and QAPI types, but I consider those libraries that might have need of such pedantic types. The iotests themselves? eh. Just figured I'd give you a range of options to choose from and you'd pick the best one. --js PS: I really really really really really wish that 3.6 had TypedDict.
© 2016 - 2026 Red Hat, Inc.