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.