[Qemu-devel] [PATCH v2 3/3] scripts: Remove debug parameter from QEMUMachine

Eduardo Habkost posted 3 patches 8 years, 4 months ago
[Qemu-devel] [PATCH v2 3/3] scripts: Remove debug parameter from QEMUMachine
Posted by Eduardo Habkost 8 years, 4 months ago
All scripts that use the QEMUMachine and QEMUQtestMachine classes
(device-crash-test, tests/migration/*, iotests.py, basevm.py)
already configure logging.

The basicConfig() call inside QEMUMachine.__init__() is being
kept just to make sure a script would still work if it didn't
configure logging.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 scripts/qemu.py                     | 6 ++----
 tests/migration/guestperf/engine.py | 6 ++----
 tests/qemu-iotests/iotests.py       | 2 --
 3 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index f6d2e68627..9bfdf6d37d 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -54,7 +54,7 @@ class QEMUMachine(object):
 
     def __init__(self, binary, args=None, wrapper=None, name=None,
                  test_dir="/var/tmp", monitor_address=None,
-                 socket_scm_helper=None, debug=False):
+                 socket_scm_helper=None):
         '''
         Initialize a QEMUMachine
 
@@ -65,7 +65,6 @@ class QEMUMachine(object):
         @param test_dir: where to create socket and log file
         @param monitor_address: address for QMP monitor
         @param socket_scm_helper: helper program, required for send_fd_scm()"
-        @param debug: enable debug mode
         @note: Qemu process is not started until launch() is used.
         '''
         if args is None:
@@ -85,12 +84,11 @@ class QEMUMachine(object):
         self._events = []
         self._iolog = None
         self._socket_scm_helper = socket_scm_helper
-        self._debug = debug
         self._qmp = None
         self._qemu_full_args = None
 
         # just in case logging wasn't configured by the main script:
-        logging.basicConfig(level=(logging.DEBUG if debug else logging.WARN))
+        logging.basicConfig()
 
     def __enter__(self):
         return self
diff --git a/tests/migration/guestperf/engine.py b/tests/migration/guestperf/engine.py
index 0a13050bc6..e14d4320b2 100644
--- a/tests/migration/guestperf/engine.py
+++ b/tests/migration/guestperf/engine.py
@@ -388,15 +388,13 @@ class Engine(object):
                                args=self._get_src_args(hardware),
                                wrapper=self._get_src_wrapper(hardware),
                                name="qemu-src-%d" % os.getpid(),
-                               monitor_address=srcmonaddr,
-                               debug=self._debug)
+                               monitor_address=srcmonaddr)
 
         dst = qemu.QEMUMachine(self._binary,
                                args=self._get_dst_args(hardware, uri),
                                wrapper=self._get_dst_wrapper(hardware),
                                name="qemu-dst-%d" % os.getpid(),
-                               monitor_address=dstmonaddr,
-                               debug=self._debug)
+                               monitor_address=dstmonaddr)
 
         try:
             src.launch()
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 36a7757aaf..6f057904a9 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -195,8 +195,6 @@ class VM(qtest.QEMUQtestMachine):
         super(VM, self).__init__(qemu_prog, qemu_opts, name=name,
                                  test_dir=test_dir,
                                  socket_scm_helper=socket_scm_helper)
-        if debug:
-            self._debug = True
         self._num_drives = 0
 
     def add_device(self, opts):
-- 
2.13.6


Re: [Qemu-devel] [PATCH v2 3/3] scripts: Remove debug parameter from QEMUMachine
Posted by Lukáš Doktor 8 years, 4 months ago
Dne 5.10.2017 v 19:20 Eduardo Habkost napsal(a):
> All scripts that use the QEMUMachine and QEMUQtestMachine classes
> (device-crash-test, tests/migration/*, iotests.py, basevm.py)
> already configure logging.
> 
> The basicConfig() call inside QEMUMachine.__init__() is being
> kept just to make sure a script would still work if it didn't
> configure logging.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  scripts/qemu.py                     | 6 ++----
>  tests/migration/guestperf/engine.py | 6 ++----
>  tests/qemu-iotests/iotests.py       | 2 --
>  3 files changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/scripts/qemu.py b/scripts/qemu.py
> index f6d2e68627..9bfdf6d37d 100644
> --- a/scripts/qemu.py
> +++ b/scripts/qemu.py
> @@ -54,7 +54,7 @@ class QEMUMachine(object):
>  
>      def __init__(self, binary, args=None, wrapper=None, name=None,
>                   test_dir="/var/tmp", monitor_address=None,
> -                 socket_scm_helper=None, debug=False):
> +                 socket_scm_helper=None):
>          '''
>          Initialize a QEMUMachine
>  
> @@ -65,7 +65,6 @@ class QEMUMachine(object):
>          @param test_dir: where to create socket and log file
>          @param monitor_address: address for QMP monitor
>          @param socket_scm_helper: helper program, required for send_fd_scm()"
> -        @param debug: enable debug mode
>          @note: Qemu process is not started until launch() is used.
>          '''
>          if args is None:
> @@ -85,12 +84,11 @@ class QEMUMachine(object):
>          self._events = []
>          self._iolog = None
>          self._socket_scm_helper = socket_scm_helper
> -        self._debug = debug
>          self._qmp = None
>          self._qemu_full_args = None
>  
>          # just in case logging wasn't configured by the main script:
> -        logging.basicConfig(level=(logging.DEBUG if debug else logging.WARN))
> +        logging.basicConfig()
Yes, this behaves the same as `debug=False`

>  
>      def __enter__(self):
>          return self
> diff --git a/tests/migration/guestperf/engine.py b/tests/migration/guestperf/engine.py
> index 0a13050bc6..e14d4320b2 100644
> --- a/tests/migration/guestperf/engine.py
> +++ b/tests/migration/guestperf/engine.py
> @@ -388,15 +388,13 @@ class Engine(object):
>                                 args=self._get_src_args(hardware),
>                                 wrapper=self._get_src_wrapper(hardware),
>                                 name="qemu-src-%d" % os.getpid(),
> -                               monitor_address=srcmonaddr,
> -                               debug=self._debug)
> +                               monitor_address=srcmonaddr)
>  
>          dst = qemu.QEMUMachine(self._binary,
>                                 args=self._get_dst_args(hardware, uri),
>                                 wrapper=self._get_dst_wrapper(hardware),
>                                 name="qemu-dst-%d" % os.getpid(),
> -                               monitor_address=dstmonaddr,
> -                               debug=self._debug)
> +                               monitor_address=dstmonaddr)
>  
>          try:
>              src.launch()
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index 36a7757aaf..6f057904a9 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -195,8 +195,6 @@ class VM(qtest.QEMUQtestMachine):
>          super(VM, self).__init__(qemu_prog, qemu_opts, name=name,
>                                   test_dir=test_dir,
>                                   socket_scm_helper=socket_scm_helper)
> -        if debug:
> -            self._debug = True

And this is the main issue. So instead of the fix I proposed in previous commit major changes to "tests/qemu-iotests/iotests.py" are necessary.

>          self._num_drives = 0
>  
>      def add_device(self, opts):
> 

Re: [Qemu-devel] [PATCH v2 3/3] scripts: Remove debug parameter from QEMUMachine
Posted by Eduardo Habkost 8 years, 4 months ago
On Sat, Oct 07, 2017 at 10:34:57AM +0200, Lukáš Doktor wrote:
> Dne 5.10.2017 v 19:20 Eduardo Habkost napsal(a):
> > All scripts that use the QEMUMachine and QEMUQtestMachine classes
> > (device-crash-test, tests/migration/*, iotests.py, basevm.py)
> > already configure logging.
> > 
> > The basicConfig() call inside QEMUMachine.__init__() is being
> > kept just to make sure a script would still work if it didn't
> > configure logging.
> > 
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> >  scripts/qemu.py                     | 6 ++----
> >  tests/migration/guestperf/engine.py | 6 ++----
> >  tests/qemu-iotests/iotests.py       | 2 --
> >  3 files changed, 4 insertions(+), 10 deletions(-)
> > 
> > diff --git a/scripts/qemu.py b/scripts/qemu.py
> > index f6d2e68627..9bfdf6d37d 100644
> > --- a/scripts/qemu.py
> > +++ b/scripts/qemu.py
> > @@ -54,7 +54,7 @@ class QEMUMachine(object):
> >  
> >      def __init__(self, binary, args=None, wrapper=None, name=None,
> >                   test_dir="/var/tmp", monitor_address=None,
> > -                 socket_scm_helper=None, debug=False):
> > +                 socket_scm_helper=None):
> >          '''
> >          Initialize a QEMUMachine
> >  
> > @@ -65,7 +65,6 @@ class QEMUMachine(object):
> >          @param test_dir: where to create socket and log file
> >          @param monitor_address: address for QMP monitor
> >          @param socket_scm_helper: helper program, required for send_fd_scm()"
> > -        @param debug: enable debug mode
> >          @note: Qemu process is not started until launch() is used.
> >          '''
> >          if args is None:
> > @@ -85,12 +84,11 @@ class QEMUMachine(object):
> >          self._events = []
> >          self._iolog = None
> >          self._socket_scm_helper = socket_scm_helper
> > -        self._debug = debug
> >          self._qmp = None
> >          self._qemu_full_args = None
> >  
> >          # just in case logging wasn't configured by the main script:
> > -        logging.basicConfig(level=(logging.DEBUG if debug else logging.WARN))
> > +        logging.basicConfig()
> Yes, this behaves the same as `debug=False`
> 
> >  
> >      def __enter__(self):
> >          return self
> > diff --git a/tests/migration/guestperf/engine.py b/tests/migration/guestperf/engine.py
> > index 0a13050bc6..e14d4320b2 100644
> > --- a/tests/migration/guestperf/engine.py
> > +++ b/tests/migration/guestperf/engine.py
> > @@ -388,15 +388,13 @@ class Engine(object):
> >                                 args=self._get_src_args(hardware),
> >                                 wrapper=self._get_src_wrapper(hardware),
> >                                 name="qemu-src-%d" % os.getpid(),
> > -                               monitor_address=srcmonaddr,
> > -                               debug=self._debug)
> > +                               monitor_address=srcmonaddr)
> >  
> >          dst = qemu.QEMUMachine(self._binary,
> >                                 args=self._get_dst_args(hardware, uri),
> >                                 wrapper=self._get_dst_wrapper(hardware),
> >                                 name="qemu-dst-%d" % os.getpid(),
> > -                               monitor_address=dstmonaddr,
> > -                               debug=self._debug)
> > +                               monitor_address=dstmonaddr)
> >  
> >          try:
> >              src.launch()
> > diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> > index 36a7757aaf..6f057904a9 100644
> > --- a/tests/qemu-iotests/iotests.py
> > +++ b/tests/qemu-iotests/iotests.py
> > @@ -195,8 +195,6 @@ class VM(qtest.QEMUQtestMachine):
> >          super(VM, self).__init__(qemu_prog, qemu_opts, name=name,
> >                                   test_dir=test_dir,
> >                                   socket_scm_helper=socket_scm_helper)
> > -        if debug:
> > -            self._debug = True
> 
> And this is the main issue. So instead of the fix I proposed in
> previous commit major changes to
> "tests/qemu-iotests/iotests.py" are necessary.

Could you clarify what those major changes are?

iotests.py was already changed to call basicConfig() according to
args.debug in main().


> 
> >          self._num_drives = 0
> >  
> >      def add_device(self, opts):
> > 
> 




-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 3/3] scripts: Remove debug parameter from QEMUMachine
Posted by Lukáš Doktor 8 years, 4 months ago
Dne 10.10.2017 v 04:50 Eduardo Habkost napsal(a):
> On Sat, Oct 07, 2017 at 10:34:57AM +0200, Lukáš Doktor wrote:
>> Dne 5.10.2017 v 19:20 Eduardo Habkost napsal(a):
>>> All scripts that use the QEMUMachine and QEMUQtestMachine classes
>>> (device-crash-test, tests/migration/*, iotests.py, basevm.py)
>>> already configure logging.
>>>
>>> The basicConfig() call inside QEMUMachine.__init__() is being
>>> kept just to make sure a script would still work if it didn't
>>> configure logging.
>>>
>>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>>> ---
>>>  scripts/qemu.py                     | 6 ++----
>>>  tests/migration/guestperf/engine.py | 6 ++----
>>>  tests/qemu-iotests/iotests.py       | 2 --
>>>  3 files changed, 4 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/scripts/qemu.py b/scripts/qemu.py
>>> index f6d2e68627..9bfdf6d37d 100644
>>> --- a/scripts/qemu.py
>>> +++ b/scripts/qemu.py
>>> @@ -54,7 +54,7 @@ class QEMUMachine(object):
>>>  
>>>      def __init__(self, binary, args=None, wrapper=None, name=None,
>>>                   test_dir="/var/tmp", monitor_address=None,
>>> -                 socket_scm_helper=None, debug=False):
>>> +                 socket_scm_helper=None):
>>>          '''
>>>          Initialize a QEMUMachine
>>>  
>>> @@ -65,7 +65,6 @@ class QEMUMachine(object):
>>>          @param test_dir: where to create socket and log file
>>>          @param monitor_address: address for QMP monitor
>>>          @param socket_scm_helper: helper program, required for send_fd_scm()"
>>> -        @param debug: enable debug mode
>>>          @note: Qemu process is not started until launch() is used.
>>>          '''
>>>          if args is None:
>>> @@ -85,12 +84,11 @@ class QEMUMachine(object):
>>>          self._events = []
>>>          self._iolog = None
>>>          self._socket_scm_helper = socket_scm_helper
>>> -        self._debug = debug
>>>          self._qmp = None
>>>          self._qemu_full_args = None
>>>  
>>>          # just in case logging wasn't configured by the main script:
>>> -        logging.basicConfig(level=(logging.DEBUG if debug else logging.WARN))
>>> +        logging.basicConfig()
>> Yes, this behaves the same as `debug=False`
>>
>>>  
>>>      def __enter__(self):
>>>          return self
>>> diff --git a/tests/migration/guestperf/engine.py b/tests/migration/guestperf/engine.py
>>> index 0a13050bc6..e14d4320b2 100644
>>> --- a/tests/migration/guestperf/engine.py
>>> +++ b/tests/migration/guestperf/engine.py
>>> @@ -388,15 +388,13 @@ class Engine(object):
>>>                                 args=self._get_src_args(hardware),
>>>                                 wrapper=self._get_src_wrapper(hardware),
>>>                                 name="qemu-src-%d" % os.getpid(),
>>> -                               monitor_address=srcmonaddr,
>>> -                               debug=self._debug)
>>> +                               monitor_address=srcmonaddr)
>>>  
>>>          dst = qemu.QEMUMachine(self._binary,
>>>                                 args=self._get_dst_args(hardware, uri),
>>>                                 wrapper=self._get_dst_wrapper(hardware),
>>>                                 name="qemu-dst-%d" % os.getpid(),
>>> -                               monitor_address=dstmonaddr,
>>> -                               debug=self._debug)
>>> +                               monitor_address=dstmonaddr)
>>>  
>>>          try:
>>>              src.launch()
>>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>>> index 36a7757aaf..6f057904a9 100644
>>> --- a/tests/qemu-iotests/iotests.py
>>> +++ b/tests/qemu-iotests/iotests.py
>>> @@ -195,8 +195,6 @@ class VM(qtest.QEMUQtestMachine):
>>>          super(VM, self).__init__(qemu_prog, qemu_opts, name=name,
>>>                                   test_dir=test_dir,
>>>                                   socket_scm_helper=socket_scm_helper)
>>> -        if debug:
>>> -            self._debug = True
>>
>> And this is the main issue. So instead of the fix I proposed in
>> previous commit major changes to
>> "tests/qemu-iotests/iotests.py" are necessary.
> 
> Could you clarify what those major changes are?
> 
> iotests.py was already changed to call basicConfig() according to
> args.debug in main().
> 

Hello Eduardo,

I'm sorry, I was not aware of that patch. I think I used git://github.com/ehabkost/qemu.git branch python-next as HEAD, but either I forgot to update it, or that patch was not yet pushed, or was I suppose to use different HEAD? I'm sorry, I'm not used to this kind of workflow.

Btw the afa79b55676dcd1859aa9d1f983c9dfbbcc13197 is exactly what I had in mind. The major changes I mentioned were related to `TestRunner` stream and it's verbosity, which was not part of that commit and is not really necessary.

Anyway with that commit this works well.

Reviewed-by: Lukáš Doktor <ldoktor@redhat.com> 

> 
>>
>>>          self._num_drives = 0
>>>  
>>>      def add_device(self, opts):
>>>
>>
> 
> 
> 
>