[PATCH v4 0/5] iotests/297: Cover tests/

[PATCH v4 0/5] iotests/297: Cover tests/

[Hanna Reitz]

v1: https://lists.nongnu.org/archive/html/qemu-block/2021-03/msg01471.html
v2: https://lists.nongnu.org/archive/html/qemu-block/2021-05/msg00492.html
v3: https://lists.nongnu.org/archive/html/qemu-block/2021-05/msg00569.html


Hi,

Sorry for the long delay, here is v4 to make our lint checking iotest
297 cover the tests/ subdirectory.


v4:
- Fixed the explanatory comment in patch 3 as suggested by Vladimir
- Added patch 4


git-backport-diff against v3:

Key:
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/5:[----] [-C] 'iotests/297: Drop 169 and 199 from the skip list'
002/5:[----] [--] 'migrate-bitmaps-postcopy-test: Fix pylint warnings'
003/5:[0006] [FC] 'migrate-bitmaps-test: Fix pylint warnings'
004/5:[down] 'mirror-top-perms: Fix AbnormalShutdown path'
005/5:[----] [--] 'iotests/297: Cover tests/'


Hanna Reitz (5):
  iotests/297: Drop 169 and 199 from the skip list
  migrate-bitmaps-postcopy-test: Fix pylint warnings
  migrate-bitmaps-test: Fix pylint warnings
  mirror-top-perms: Fix AbnormalShutdown path
  iotests/297: Cover tests/

 tests/qemu-iotests/297                        |  7 +--
 .../tests/migrate-bitmaps-postcopy-test       | 13 +++---
 tests/qemu-iotests/tests/migrate-bitmaps-test | 43 +++++++++++--------
 tests/qemu-iotests/tests/mirror-top-perms     |  2 +-
 4 files changed, 37 insertions(+), 28 deletions(-)

-- 
2.31.1

[PATCH v4 1/5] iotests/297: Drop 169 and 199 from the skip list

[Hanna Reitz]

169 and 199 have been renamed and moved to tests/ (commit a44be0334be:
"iotests: rename and move 169 and 199 tests"), so we can drop them from
the skip list.

Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Reviewed-by: Willian Rampazzo <willianr@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/297 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index 345b617b34..c7d709cf50 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -29,7 +29,7 @@ import iotests
 SKIP_FILES = (
     '030', '040', '041', '044', '045', '055', '056', '057', '065', '093',
     '096', '118', '124', '132', '136', '139', '147', '148', '149',
-    '151', '152', '155', '163', '165', '169', '194', '196', '199', '202',
+    '151', '152', '155', '163', '165', '194', '196', '202',
     '203', '205', '206', '207', '208', '210', '211', '212', '213', '216',
     '218', '219', '224', '228', '234', '235', '236', '237', '238',
     '240', '242', '245', '246', '248', '255', '256', '257', '258', '260',
-- 
2.31.1

[PATCH v4 2/5] migrate-bitmaps-postcopy-test: Fix pylint warnings

[Hanna Reitz]

pylint complains that discards1_sha256 and all_discards_sha256 are first
set in non-__init__ methods.

These variables are not really class-variables anyway, so let them
instead be returned by start_postcopy(), thus silencing pylint.

Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 .../tests/migrate-bitmaps-postcopy-test             | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test b/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test
index 584062b412..00ebb5c251 100755
--- a/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test
+++ b/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test
@@ -132,10 +132,10 @@ class TestDirtyBitmapPostcopyMigration(iotests.QMPTestCase):
 
         result = self.vm_a.qmp('x-debug-block-dirty-bitmap-sha256',
                                node='drive0', name='bitmap0')
-        self.discards1_sha256 = result['return']['sha256']
+        discards1_sha256 = result['return']['sha256']
 
         # Check, that updating the bitmap by discards works
-        assert self.discards1_sha256 != empty_sha256
+        assert discards1_sha256 != empty_sha256
 
         # We want to calculate resulting sha256. Do it in bitmap0, so, disable
         # other bitmaps
@@ -148,7 +148,7 @@ class TestDirtyBitmapPostcopyMigration(iotests.QMPTestCase):
 
         result = self.vm_a.qmp('x-debug-block-dirty-bitmap-sha256',
                                node='drive0', name='bitmap0')
-        self.all_discards_sha256 = result['return']['sha256']
+        all_discards_sha256 = result['return']['sha256']
 
         # Now, enable some bitmaps, to be updated during migration
         for i in range(2, nb_bitmaps, 2):
@@ -173,10 +173,11 @@ class TestDirtyBitmapPostcopyMigration(iotests.QMPTestCase):
 
         event_resume = self.vm_b.event_wait('RESUME')
         self.vm_b_events.append(event_resume)
-        return event_resume
+        return (event_resume, discards1_sha256, all_discards_sha256)
 
     def test_postcopy_success(self):
-        event_resume = self.start_postcopy()
+        event_resume, discards1_sha256, all_discards_sha256 = \
+                self.start_postcopy()
 
         # enabled bitmaps should be updated
         apply_discards(self.vm_b, discards2)
@@ -217,7 +218,7 @@ class TestDirtyBitmapPostcopyMigration(iotests.QMPTestCase):
         for i in range(0, nb_bitmaps, 5):
             result = self.vm_b.qmp('x-debug-block-dirty-bitmap-sha256',
                                    node='drive0', name='bitmap{}'.format(i))
-            sha = self.discards1_sha256 if i % 2 else self.all_discards_sha256
+            sha = discards1_sha256 if i % 2 else all_discards_sha256
             self.assert_qmp(result, 'return/sha256', sha)
 
     def test_early_shutdown_destination(self):
-- 
2.31.1

[PATCH v4 3/5] migrate-bitmaps-test: Fix pylint warnings

[Hanna Reitz]

There are a couple of things pylint takes issue with:
- The "time" import is unused
- The import order (iotests should come last)
- get_bitmap_hash() doesn't use @self and so should be a function
- Semicolons at the end of some lines
- Parentheses after "if"
- Some lines are too long (80 characters instead of 79)
- inject_test_case()'s @name parameter shadows a top-level @name
  variable
- "lambda self: mc(self)" were equivalent to just "mc", but in
  inject_test_case(), it is not equivalent, so add a comment and disable
  the warning locally
- Always put two empty lines after a function
- f'exec: cat > /dev/null' does not need to be an f-string

Fix them.

Signed-off-by: Hanna Reitz <hreitz@redhat.com>
---
 tests/qemu-iotests/tests/migrate-bitmaps-test | 43 +++++++++++--------
 1 file changed, 25 insertions(+), 18 deletions(-)

diff --git a/tests/qemu-iotests/tests/migrate-bitmaps-test b/tests/qemu-iotests/tests/migrate-bitmaps-test
index a5c7bc83e0..dc431c35b3 100755
--- a/tests/qemu-iotests/tests/migrate-bitmaps-test
+++ b/tests/qemu-iotests/tests/migrate-bitmaps-test
@@ -20,11 +20,10 @@
 #
 
 import os
-import iotests
-import time
 import itertools
 import operator
 import re
+import iotests
 from iotests import qemu_img, qemu_img_create, Timeout
 
 
@@ -37,6 +36,12 @@ mig_cmd = 'exec: cat > ' + mig_file
 incoming_cmd = 'exec: cat ' + mig_file
 
 
+def get_bitmap_hash(vm):
+    result = vm.qmp('x-debug-block-dirty-bitmap-sha256',
+                    node='drive0', name='bitmap0')
+    return result['return']['sha256']
+
+
 class TestDirtyBitmapMigration(iotests.QMPTestCase):
     def tearDown(self):
         self.vm_a.shutdown()
@@ -62,21 +67,16 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
             params['persistent'] = True
 
         result = vm.qmp('block-dirty-bitmap-add', **params)
-        self.assert_qmp(result, 'return', {});
-
-    def get_bitmap_hash(self, vm):
-        result = vm.qmp('x-debug-block-dirty-bitmap-sha256',
-                        node='drive0', name='bitmap0')
-        return result['return']['sha256']
+        self.assert_qmp(result, 'return', {})
 
     def check_bitmap(self, vm, sha256):
         result = vm.qmp('x-debug-block-dirty-bitmap-sha256',
                         node='drive0', name='bitmap0')
         if sha256:
-            self.assert_qmp(result, 'return/sha256', sha256);
+            self.assert_qmp(result, 'return/sha256', sha256)
         else:
             self.assert_qmp(result, 'error/desc',
-                            "Dirty bitmap 'bitmap0' not found");
+                            "Dirty bitmap 'bitmap0' not found")
 
     def do_test_migration_resume_source(self, persistent, migrate_bitmaps):
         granularity = 512
@@ -97,7 +97,7 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
         self.add_bitmap(self.vm_a, granularity, persistent)
         for r in regions:
             self.vm_a.hmp_qemu_io('drive0', 'write %d %d' % r)
-        sha256 = self.get_bitmap_hash(self.vm_a)
+        sha256 = get_bitmap_hash(self.vm_a)
 
         result = self.vm_a.qmp('migrate', uri=mig_cmd)
         while True:
@@ -106,7 +106,7 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
                 break
         while True:
             result = self.vm_a.qmp('query-status')
-            if (result['return']['status'] == 'postmigrate'):
+            if result['return']['status'] == 'postmigrate':
                 break
 
         # test that bitmap is still here
@@ -164,7 +164,7 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
         self.add_bitmap(self.vm_a, granularity, persistent)
         for r in regions:
             self.vm_a.hmp_qemu_io('drive0', 'write %d %d' % r)
-        sha256 = self.get_bitmap_hash(self.vm_a)
+        sha256 = get_bitmap_hash(self.vm_a)
 
         if pre_shutdown:
             self.vm_a.shutdown()
@@ -214,16 +214,22 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
             self.check_bitmap(self.vm_b, sha256 if persistent else False)
 
 
-def inject_test_case(klass, name, method, *args, **kwargs):
+def inject_test_case(klass, suffix, method, *args, **kwargs):
     mc = operator.methodcaller(method, *args, **kwargs)
-    setattr(klass, 'test_' + method + name, lambda self: mc(self))
+    # We want to add a function attribute to `klass`, so that it is
+    # correctly converted to a method on instantiation.  The
+    # methodcaller object `mc` is a callable, not a function, so we
+    # need the lambda to turn it into a function.
+    # pylint: disable=unnecessary-lambda
+    setattr(klass, 'test_' + method + suffix, lambda self: mc(self))
+
 
 for cmb in list(itertools.product((True, False), repeat=5)):
     name = ('_' if cmb[0] else '_not_') + 'persistent_'
     name += ('_' if cmb[1] else '_not_') + 'migbitmap_'
     name += '_online' if cmb[2] else '_offline'
     name += '_shared' if cmb[3] else '_nonshared'
-    if (cmb[4]):
+    if cmb[4]:
         name += '__pre_shutdown'
 
     inject_test_case(TestDirtyBitmapMigration, name, 'do_test_migration',
@@ -270,7 +276,8 @@ class TestDirtyBitmapBackingMigration(iotests.QMPTestCase):
         self.assert_qmp(result, 'return', {})
 
         # Check that the bitmaps are there
-        for node in self.vm.qmp('query-named-block-nodes', flat=True)['return']:
+        nodes = self.vm.qmp('query-named-block-nodes', flat=True)['return']
+        for node in nodes:
             if 'node0' in node['node-name']:
                 self.assert_qmp(node, 'dirty-bitmaps[0]/name', 'bmap0')
 
@@ -287,7 +294,7 @@ class TestDirtyBitmapBackingMigration(iotests.QMPTestCase):
         """
         Continue the source after migration.
         """
-        result = self.vm.qmp('migrate', uri=f'exec: cat > /dev/null')
+        result = self.vm.qmp('migrate', uri='exec: cat > /dev/null')
         self.assert_qmp(result, 'return', {})
 
         with Timeout(10, 'Migration timeout'):
-- 
2.31.1

[PATCH v4 4/5] mirror-top-perms: Fix AbnormalShutdown path

[Hanna Reitz]

The AbnormalShutdown exception class is not in qemu.machine, but in
qemu.machine.machine.  (qemu.machine.AbnormalShutdown was enough for
Python to find it in order to run this test, but pylint complains about
it.)

Signed-off-by: Hanna Reitz <hreitz@redhat.com>
---
 tests/qemu-iotests/tests/mirror-top-perms | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/tests/mirror-top-perms b/tests/qemu-iotests/tests/mirror-top-perms
index 451a0666f8..2fc8dd66e0 100755
--- a/tests/qemu-iotests/tests/mirror-top-perms
+++ b/tests/qemu-iotests/tests/mirror-top-perms
@@ -47,7 +47,7 @@ class TestMirrorTopPerms(iotests.QMPTestCase):
     def tearDown(self):
         try:
             self.vm.shutdown()
-        except qemu.machine.AbnormalShutdown:
+        except qemu.machine.machine.AbnormalShutdown:
             pass
 
         if self.vm_b is not None:
-- 
2.31.1

[PATCH v4 5/5] iotests/297: Cover tests/

[Hanna Reitz]

297 so far does not check the named tests, which reside in the tests/
directory (i.e. full path tests/qemu-iotests/tests).  Fix it.

Thanks to the previous two commits, all named tests pass its scrutiny,
so we do not have to add anything to SKIP_FILES.

Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Reviewed-by: Willian Rampazzo <willianr@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/297 | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index c7d709cf50..ac10bd1e1a 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -55,8 +55,9 @@ def is_python_file(filename):
 
 
 def run_linters():
-    files = [filename for filename in (set(os.listdir('.')) - set(SKIP_FILES))
-             if is_python_file(filename)]
+    named_tests = [f'tests/{entry}' for entry in os.listdir('tests')]
+    check_tests = set(os.listdir('.') + named_tests) - set(SKIP_FILES)
+    files = [filename for filename in check_tests if is_python_file(filename)]
 
     iotests.logger.debug('Files to be checked:')
     iotests.logger.debug(', '.join(sorted(files)))
-- 
2.31.1

Re: [PATCH v4 3/5] migrate-bitmaps-test: Fix pylint warnings

[Vladimir Sementsov-Ogievskiy]

02.09.2021 12:40, Hanna Reitz wrote:
> There are a couple of things pylint takes issue with:
> - The "time" import is unused
> - The import order (iotests should come last)
> - get_bitmap_hash() doesn't use @self and so should be a function
> - Semicolons at the end of some lines
> - Parentheses after "if"
> - Some lines are too long (80 characters instead of 79)
> - inject_test_case()'s @name parameter shadows a top-level @name
>    variable
> - "lambda self: mc(self)" were equivalent to just "mc", but in
>    inject_test_case(), it is not equivalent, so add a comment and disable
>    the warning locally
> - Always put two empty lines after a function
> - f'exec: cat > /dev/null' does not need to be an f-string
> 
> Fix them.
> 
> Signed-off-by: Hanna Reitz<hreitz@redhat.com>

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

-- 
Best regards,
Vladimir

Re: [PATCH v4 4/5] mirror-top-perms: Fix AbnormalShutdown path

[Vladimir Sementsov-Ogievskiy]

02.09.2021 12:40, Hanna Reitz wrote:
> The AbnormalShutdown exception class is not in qemu.machine, but in
> qemu.machine.machine.  (qemu.machine.AbnormalShutdown was enough for
> Python to find it in order to run this test, but pylint complains about
> it.)
> 
> Signed-off-by: Hanna Reitz <hreitz@redhat.com>
> ---
>   tests/qemu-iotests/tests/mirror-top-perms | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/qemu-iotests/tests/mirror-top-perms b/tests/qemu-iotests/tests/mirror-top-perms
> index 451a0666f8..2fc8dd66e0 100755
> --- a/tests/qemu-iotests/tests/mirror-top-perms
> +++ b/tests/qemu-iotests/tests/mirror-top-perms
> @@ -47,7 +47,7 @@ class TestMirrorTopPerms(iotests.QMPTestCase):
>       def tearDown(self):
>           try:
>               self.vm.shutdown()
> -        except qemu.machine.AbnormalShutdown:
> +        except qemu.machine.machine.AbnormalShutdown:
>               pass
>   
>           if self.vm_b is not None:
> 

Hmm, interesting.. May be that bad that module has same name as subpackage?

Anyway, I don't care too much. Interesting how Python find it o_O.

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

-- 
Best regards,
Vladimir

Re: [PATCH v4 4/5] mirror-top-perms: Fix AbnormalShutdown path

[Philippe Mathieu-Daudé]

On 9/2/21 11:58 AM, Vladimir Sementsov-Ogievskiy wrote:
> 02.09.2021 12:40, Hanna Reitz wrote:
>> The AbnormalShutdown exception class is not in qemu.machine, but in
>> qemu.machine.machine.  (qemu.machine.AbnormalShutdown was enough for
>> Python to find it in order to run this test, but pylint complains about
>> it.)
>>
>> Signed-off-by: Hanna Reitz <hreitz@redhat.com>
>> ---
>>   tests/qemu-iotests/tests/mirror-top-perms | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tests/qemu-iotests/tests/mirror-top-perms
>> b/tests/qemu-iotests/tests/mirror-top-perms
>> index 451a0666f8..2fc8dd66e0 100755
>> --- a/tests/qemu-iotests/tests/mirror-top-perms
>> +++ b/tests/qemu-iotests/tests/mirror-top-perms
>> @@ -47,7 +47,7 @@ class TestMirrorTopPerms(iotests.QMPTestCase):
>>       def tearDown(self):
>>           try:
>>               self.vm.shutdown()
>> -        except qemu.machine.AbnormalShutdown:
>> +        except qemu.machine.machine.AbnormalShutdown:
>>               pass
>>             if self.vm_b is not None:
>>
> 
> Hmm, interesting.. May be that bad that module has same name as subpackage?

Confusing indeed. Could this be improved?

Re: [PATCH v4 0/5] iotests/297: Cover tests/

[Hanna Reitz]

On 02.09.21 11:40, Hanna Reitz wrote:
> v1: https://lists.nongnu.org/archive/html/qemu-block/2021-03/msg01471.html
> v2: https://lists.nongnu.org/archive/html/qemu-block/2021-05/msg00492.html
> v3: https://lists.nongnu.org/archive/html/qemu-block/2021-05/msg00569.html
>
>
> Hi,
>
> Sorry for the long delay, here is v4 to make our lint checking iotest
> 297 cover the tests/ subdirectory.

Thanks for the review, applied to my block branch:

https://github.com/XanClic/qemu/commits/block

Hanna

Re: [PATCH v4 4/5] mirror-top-perms: Fix AbnormalShutdown path

[Hanna Reitz]

On 02.09.21 12:15, Philippe Mathieu-Daudé wrote:
> On 9/2/21 11:58 AM, Vladimir Sementsov-Ogievskiy wrote:
>> 02.09.2021 12:40, Hanna Reitz wrote:
>>> The AbnormalShutdown exception class is not in qemu.machine, but in
>>> qemu.machine.machine.  (qemu.machine.AbnormalShutdown was enough for
>>> Python to find it in order to run this test, but pylint complains about
>>> it.)
>>>
>>> Signed-off-by: Hanna Reitz <hreitz@redhat.com>
>>> ---
>>>    tests/qemu-iotests/tests/mirror-top-perms | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/tests/qemu-iotests/tests/mirror-top-perms
>>> b/tests/qemu-iotests/tests/mirror-top-perms
>>> index 451a0666f8..2fc8dd66e0 100755
>>> --- a/tests/qemu-iotests/tests/mirror-top-perms
>>> +++ b/tests/qemu-iotests/tests/mirror-top-perms
>>> @@ -47,7 +47,7 @@ class TestMirrorTopPerms(iotests.QMPTestCase):
>>>        def tearDown(self):
>>>            try:
>>>                self.vm.shutdown()
>>> -        except qemu.machine.AbnormalShutdown:
>>> +        except qemu.machine.machine.AbnormalShutdown:
>>>                pass
>>>              if self.vm_b is not None:
>>>
>> Hmm, interesting.. May be that bad that module has same name as subpackage?
> Confusing indeed. Could this be improved?

I think if we want to improve something, it would be that we make the 
exception public in the qemu.machine namespace, like so:

diff --git a/python/qemu/machine/__init__.py 
b/python/qemu/machine/__init__.py
index 9ccd58ef14..48bbb0530b 100644
--- a/python/qemu/machine/__init__.py
+++ b/python/qemu/machine/__init__.py
@@ -25,7 +25,7 @@
  # pylint: disable=import-error
  # see: https://github.com/PyCQA/pylint/issues/3624
  # see: https://github.com/PyCQA/pylint/issues/3651
-from .machine import QEMUMachine
+from .machine import AbnormalShutdown, QEMUMachine
  from .qtest import QEMUQtestMachine, QEMUQtestProtocol

But, well.  I don’t mind a qemu.machine.machine too much, personally.

Hanna

Re: [PATCH v4 4/5] mirror-top-perms: Fix AbnormalShutdown path

[Philippe Mathieu-Daudé]

On 9/7/21 11:57 AM, Hanna Reitz wrote:
> On 02.09.21 12:15, Philippe Mathieu-Daudé wrote:
>> On 9/2/21 11:58 AM, Vladimir Sementsov-Ogievskiy wrote:
>>> 02.09.2021 12:40, Hanna Reitz wrote:
>>>> The AbnormalShutdown exception class is not in qemu.machine, but in
>>>> qemu.machine.machine.  (qemu.machine.AbnormalShutdown was enough for
>>>> Python to find it in order to run this test, but pylint complains about
>>>> it.)
>>>>
>>>> Signed-off-by: Hanna Reitz <hreitz@redhat.com>
>>>> ---
>>>>    tests/qemu-iotests/tests/mirror-top-perms | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/tests/qemu-iotests/tests/mirror-top-perms
>>>> b/tests/qemu-iotests/tests/mirror-top-perms
>>>> index 451a0666f8..2fc8dd66e0 100755
>>>> --- a/tests/qemu-iotests/tests/mirror-top-perms
>>>> +++ b/tests/qemu-iotests/tests/mirror-top-perms
>>>> @@ -47,7 +47,7 @@ class TestMirrorTopPerms(iotests.QMPTestCase):
>>>>        def tearDown(self):
>>>>            try:
>>>>                self.vm.shutdown()
>>>> -        except qemu.machine.AbnormalShutdown:
>>>> +        except qemu.machine.machine.AbnormalShutdown:
>>>>                pass
>>>>              if self.vm_b is not None:
>>>>
>>> Hmm, interesting.. May be that bad that module has same name as
>>> subpackage?
>> Confusing indeed. Could this be improved?
> 
> I think if we want to improve something, it would be that we make the
> exception public in the qemu.machine namespace, like so:
> 
> diff --git a/python/qemu/machine/__init__.py
> b/python/qemu/machine/__init__.py
> index 9ccd58ef14..48bbb0530b 100644
> --- a/python/qemu/machine/__init__.py
> +++ b/python/qemu/machine/__init__.py
> @@ -25,7 +25,7 @@
>  # pylint: disable=import-error
>  # see: https://github.com/PyCQA/pylint/issues/3624
>  # see: https://github.com/PyCQA/pylint/issues/3651
> -from .machine import QEMUMachine
> +from .machine import AbnormalShutdown, QEMUMachine
>  from .qtest import QEMUQtestMachine, QEMUQtestProtocol
> 
> But, well.  I don’t mind a qemu.machine.machine too much, personally.

I'm not worried about you, but the newcomer willing to use this
interface ;) Note I'm not asking you to clean that neither, I
was just wondering why we have machine.machine.