diff mbox series

[RFC,v2,02/11] python: qemu: pass the wrapper field from QEMUQtestmachine to QEMUMachine

Message ID 20210407135021.48330-3-eesposit@redhat.com (mailing list archive)
State New, archived
Headers show
Series qemu_iotests: improve debugging options | expand

Commit Message

Emanuele Giuseppe Esposito April 7, 2021, 1:50 p.m. UTC
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 python/qemu/machine.py | 2 +-
 python/qemu/qtest.py   | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

Comments

John Snow April 8, 2021, 7:59 p.m. UTC | #1
On 4/7/21 9:50 AM, Emanuele Giuseppe Esposito wrote:
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> ---
>   python/qemu/machine.py | 2 +-
>   python/qemu/qtest.py   | 4 +++-
>   2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/python/qemu/machine.py b/python/qemu/machine.py
> index c721e07d63..18d32ebe45 100644
> --- a/python/qemu/machine.py
> +++ b/python/qemu/machine.py
> @@ -109,7 +109,7 @@ def __init__(self,
>   
>           self._binary = binary
>           self._args = list(args)
> -        self._wrapper = wrapper
> +        self._wrapper = list(wrapper)
>  

Unrelated change?

(I'm assuming you want to copy the user's input to explicitly avoid 
sharing state. Commit message blurb for this would be good.)

>           self._name = name or "qemu-%d" % os.getpid()
>           self._test_dir = test_dir
> diff --git a/python/qemu/qtest.py b/python/qemu/qtest.py
> index 0d01715086..4c90daf430 100644
> --- a/python/qemu/qtest.py
> +++ b/python/qemu/qtest.py
> @@ -111,6 +111,7 @@ class QEMUQtestMachine(QEMUMachine):
>       def __init__(self,
>                    binary: str,
>                    args: Sequence[str] = (),
> +                 wrapper: Sequence[str] = (),
>                    name: Optional[str] = None,
>                    test_dir: str = "/var/tmp",
>                    socket_scm_helper: Optional[str] = None,
> @@ -119,7 +120,8 @@ def __init__(self,
>               name = "qemu-%d" % os.getpid()
>           if sock_dir is None:
>               sock_dir = test_dir
> -        super().__init__(binary, args, name=name, test_dir=test_dir,
> +        super().__init__(binary, args, wrapper=wrapper, name=name,
> +                         test_dir=test_dir,
>                            socket_scm_helper=socket_scm_helper,
>                            sock_dir=sock_dir)
>           self._qtest: Optional[QEMUQtestProtocol] = None
> 

ACK
Emanuele Giuseppe Esposito April 9, 2021, 4:07 p.m. UTC | #2
>> diff --git a/python/qemu/machine.py b/python/qemu/machine.py
>> index c721e07d63..18d32ebe45 100644
>> --- a/python/qemu/machine.py
>> +++ b/python/qemu/machine.py
>> @@ -109,7 +109,7 @@ def __init__(self,
>>           self._binary = binary
>>           self._args = list(args)
>> -        self._wrapper = wrapper
>> +        self._wrapper = list(wrapper)
>>
> 
> Unrelated change?
> 
> (I'm assuming you want to copy the user's input to explicitly avoid 
> sharing state. Commit message blurb for this would be good.)

Yes, unrelated change. I do not see the benefit of copying the user 
state. I will drop it.

>>
> 
> ACK

(Apologies for the ignorance, is this an Acked-by?)

Emanuele
John Snow April 9, 2021, 4:37 p.m. UTC | #3
On 4/9/21 12:07 PM, Emanuele Giuseppe Esposito wrote:
> 
>>> diff --git a/python/qemu/machine.py b/python/qemu/machine.py
>>> index c721e07d63..18d32ebe45 100644
>>> --- a/python/qemu/machine.py
>>> +++ b/python/qemu/machine.py
>>> @@ -109,7 +109,7 @@ def __init__(self,
>>>           self._binary = binary
>>>           self._args = list(args)
>>> -        self._wrapper = wrapper
>>> +        self._wrapper = list(wrapper)
>>>
>>
>> Unrelated change?
>>
>> (I'm assuming you want to copy the user's input to explicitly avoid 
>> sharing state. Commit message blurb for this would be good.)
> 
> Yes, unrelated change. I do not see the benefit of copying the user 
> state. I will drop it.
> 
>>>
>>
>> ACK
> 
> (Apologies for the ignorance, is this an Acked-by?)
> 
> Emanuele
> 

Ah, yes. I just mean to say: "Minor style stuff, but looks good to me 
and I'll [likely] approve the non-RFC versions of these."

(Pending tests passing via flake8/pylint/mypy. Sorry those aren't 
formalized in the tree yet.)

--js
diff mbox series

Patch

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index c721e07d63..18d32ebe45 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -109,7 +109,7 @@  def __init__(self,
 
         self._binary = binary
         self._args = list(args)
-        self._wrapper = wrapper
+        self._wrapper = list(wrapper)
 
         self._name = name or "qemu-%d" % os.getpid()
         self._test_dir = test_dir
diff --git a/python/qemu/qtest.py b/python/qemu/qtest.py
index 0d01715086..4c90daf430 100644
--- a/python/qemu/qtest.py
+++ b/python/qemu/qtest.py
@@ -111,6 +111,7 @@  class QEMUQtestMachine(QEMUMachine):
     def __init__(self,
                  binary: str,
                  args: Sequence[str] = (),
+                 wrapper: Sequence[str] = (),
                  name: Optional[str] = None,
                  test_dir: str = "/var/tmp",
                  socket_scm_helper: Optional[str] = None,
@@ -119,7 +120,8 @@  def __init__(self,
             name = "qemu-%d" % os.getpid()
         if sock_dir is None:
             sock_dir = test_dir
-        super().__init__(binary, args, name=name, test_dir=test_dir,
+        super().__init__(binary, args, wrapper=wrapper, name=name,
+                         test_dir=test_dir,
                          socket_scm_helper=socket_scm_helper,
                          sock_dir=sock_dir)
         self._qtest: Optional[QEMUQtestProtocol] = None