Re: [Qemu-devel] [PATCH v2 1/3] guestperf: Configure logging on all shell frontends

["Lukáš Doktor" <ldoktor@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 4460 bytes --]

Dne 5.10.2017 v 19:20 Eduardo Habkost napsal(a):
> The logging module will eventually replace the 'debug' parameter
> in QEMUMachine and QEMUMonitorProtocol.
> 
> Cc: Daniel P. Berrange <berrange@redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> Changes v1 -> v2:
> * Inline init_logging() method on all callers because not all
>   classes derive from BaseShell (reported by Lukáš Doktor)
> ---
>  tests/migration/guestperf/shell.py | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/tests/migration/guestperf/shell.py b/tests/migration/guestperf/shell.py
> index 7992459a97..b272978f47 100644
> --- a/tests/migration/guestperf/shell.py
> +++ b/tests/migration/guestperf/shell.py
> @@ -26,6 +26,7 @@ sys.path.append(os.path.join(os.path.dirname(__file__),
>  import argparse
>  import fnmatch
>  import platform
> +import logging
>  
>  from guestperf.hardware import Hardware
>  from guestperf.engine import Engine
> @@ -147,6 +148,10 @@ class Shell(BaseShell):
>  
>      def run(self, argv):
>          args = self._parser.parse_args(argv)
> +        logging.basicConfig(level=(logging.DEBUG if args.debug else
> +                                   logging.INFO if args.verbose else
> +                                   logging.WARN))
> +
>  
>          engine = self.get_engine(args)
>          hardware = self.get_hardware(args)
> @@ -179,6 +184,10 @@ class BatchShell(BaseShell):
>  
>      def run(self, argv):
>          args = self._parser.parse_args(argv)
> +        logging.basicConfig(level=(logging.DEBUG if args.debug else
> +                                   logging.INFO if args.verbose else
> +                                   logging.WARN))
> +
>  
>          engine = self.get_engine(args)
>          hardware = self.get_hardware(args)
> @@ -231,6 +240,10 @@ class PlotShell(object):
>  
>      def run(self, argv):
>          args = self._parser.parse_args(argv)
> +        logging.basicConfig(level=(logging.DEBUG if args.debug else
> +                                   logging.INFO if args.verbose else
> +                                   logging.WARN))
> +
>  
>          if len(args.reports) == 0:
>              print >>sys.stderr, "At least one report required"
> 


Yep, this does the trick, also using a shared function would be IMO better (especially if we need to tweak the setup), something like

```diff
diff --git a/tests/migration/guestperf/shell.py b/tests/migration/guestperf/shell.py
index b272978..c1108ae 100644
--- a/tests/migration/guestperf/shell.py
+++ b/tests/migration/guestperf/shell.py
@@ -36,6 +36,12 @@ from guestperf.plot import Plot
 from guestperf.report import Report
 
 
+def _init_logging(args):
+    logging.basicConfig(level=(logging.DEBUG if args.debug else
+                               logging.INFO if args.verbose else
+                               logging.WARN))
+
+
 class BaseShell(object):
 
     def __init__(self):
@@ -148,10 +154,7 @@ class Shell(BaseShell):
 
     def run(self, argv):
         args = self._parser.parse_args(argv)
-        logging.basicConfig(level=(logging.DEBUG if args.debug else
-                                   logging.INFO if args.verbose else
-                                   logging.WARN))
-
+        _init_logging(args)
 
         engine = self.get_engine(args)
         hardware = self.get_hardware(args)
@@ -184,10 +187,7 @@ class BatchShell(BaseShell):
 
     def run(self, argv):
         args = self._parser.parse_args(argv)
-        logging.basicConfig(level=(logging.DEBUG if args.debug else
-                                   logging.INFO if args.verbose else
-                                   logging.WARN))
-
+        _init_logging(args)
 
         engine = self.get_engine(args)
         hardware = self.get_hardware(args)
@@ -240,10 +240,7 @@ class PlotShell(object):
 
     def run(self, argv):
         args = self._parser.parse_args(argv)
-        logging.basicConfig(level=(logging.DEBUG if args.debug else
-                                   logging.INFO if args.verbose else
-                                   logging.WARN))
-
+        _init_logging(args)
 
         if len(args.reports) == 0:
             print >>sys.stderr, "At least one report required"
```

Anyway both versions are fine by me.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 502 bytes --]