* [Qemu-devel] [PATCH v6 01/10] qemu.py: Pylint/style fixes
2017-08-18 14:26 [Qemu-devel] [PATCH v6 00/10] qemu.py: Pylint/style fixes Lukáš Doktor
@ 2017-08-18 14:26 ` Lukáš Doktor
2017-08-24 22:38 ` Cleber Rosa
2017-08-18 14:26 ` [Qemu-devel] [PATCH v6 02/10] qemu|qtest: Avoid dangerous arguments Lukáš Doktor
` (10 subsequent siblings)
11 siblings, 1 reply; 25+ messages in thread
From: Lukáš Doktor @ 2017-08-18 14:26 UTC (permalink / raw)
To: qemu-devel; +Cc: ldoktor, famz, ehabkost, apahim, armbru, mreitz, jsnow, f4bug
No actual code changes, just several pylint/style fixes and docstring
clarifications.
Signed-off-by: Lukáš Doktor <ldoktor@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
scripts/qemu.py | 73 +++++++++++++++++++++++++++++++++++++++++++--------------
1 file changed, 55 insertions(+), 18 deletions(-)
diff --git a/scripts/qemu.py b/scripts/qemu.py
index 880e3e8..dd679f1 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -23,8 +23,22 @@ import qmp.qmp
class QEMUMachine(object):
'''A QEMU VM'''
- def __init__(self, binary, args=[], wrapper=[], name=None, test_dir="/var/tmp",
- monitor_address=None, socket_scm_helper=None, debug=False):
+ def __init__(self, binary, args=[], wrapper=[], name=None,
+ test_dir="/var/tmp", monitor_address=None,
+ socket_scm_helper=None, debug=False):
+ '''
+ Initialize a QEMUMachine
+
+ @param binary: path to the qemu binary
+ @param args: list of extra arguments
+ @param wrapper: list of arguments used as prefix to qemu binary
+ @param name: prefix for socket and log file names (default: qemu-PID)
+ @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 name is None:
name = "qemu-%d" % os.getpid()
if monitor_address is None:
@@ -33,12 +47,13 @@ class QEMUMachine(object):
self._qemu_log_path = os.path.join(test_dir, name + ".log")
self._popen = None
self._binary = binary
- self._args = list(args) # Force copy args in case we modify them
+ self._args = list(args) # Force copy args in case we modify them
self._wrapper = wrapper
self._events = []
self._iolog = None
self._socket_scm_helper = socket_scm_helper
self._debug = debug
+ self._qmp = None
# This can be used to add an unused monitor instance.
def add_monitor_telnet(self, ip, port):
@@ -64,16 +79,16 @@ class QEMUMachine(object):
if self._socket_scm_helper is None:
print >>sys.stderr, "No path to socket_scm_helper set"
return -1
- if os.path.exists(self._socket_scm_helper) == False:
+ if not os.path.exists(self._socket_scm_helper):
print >>sys.stderr, "%s does not exist" % self._socket_scm_helper
return -1
fd_param = ["%s" % self._socket_scm_helper,
"%d" % self._qmp.get_sock_fd(),
"%s" % fd_file_path]
devnull = open('/dev/null', 'rb')
- p = subprocess.Popen(fd_param, stdin=devnull, stdout=sys.stdout,
- stderr=sys.stderr)
- return p.wait()
+ proc = subprocess.Popen(fd_param, stdin=devnull, stdout=sys.stdout,
+ stderr=sys.stderr)
+ return proc.wait()
@staticmethod
def _remove_if_exists(path):
@@ -99,8 +114,8 @@ class QEMUMachine(object):
return self._popen.pid
def _load_io_log(self):
- with open(self._qemu_log_path, "r") as fh:
- self._iolog = fh.read()
+ with open(self._qemu_log_path, "r") as iolog:
+ self._iolog = iolog.read()
def _base_args(self):
if isinstance(self._monitor_address, tuple):
@@ -114,7 +129,8 @@ class QEMUMachine(object):
'-display', 'none', '-vga', 'none']
def _pre_launch(self):
- self._qmp = qmp.qmp.QEMUMonitorProtocol(self._monitor_address, server=True,
+ self._qmp = qmp.qmp.QEMUMonitorProtocol(self._monitor_address,
+ server=True,
debug=self._debug)
def _post_launch(self):
@@ -131,9 +147,11 @@ class QEMUMachine(object):
qemulog = open(self._qemu_log_path, 'wb')
try:
self._pre_launch()
- args = self._wrapper + [self._binary] + self._base_args() + self._args
+ args = (self._wrapper + [self._binary] + self._base_args() +
+ self._args)
self._popen = subprocess.Popen(args, stdin=devnull, stdout=qemulog,
- stderr=subprocess.STDOUT, shell=False)
+ stderr=subprocess.STDOUT,
+ shell=False)
self._post_launch()
except:
if self.is_running():
@@ -154,23 +172,30 @@ class QEMUMachine(object):
exitcode = self._popen.wait()
if exitcode < 0:
- sys.stderr.write('qemu received signal %i: %s\n' % (-exitcode, ' '.join(self._args)))
+ sys.stderr.write('qemu received signal %i: %s\n'
+ % (-exitcode, ' '.join(self._args)))
self._load_io_log()
self._post_shutdown()
underscore_to_dash = string.maketrans('_', '-')
+
def qmp(self, cmd, conv_keys=True, **args):
- '''Invoke a QMP command and return the result dict'''
+ '''Invoke a QMP command and return the response dict'''
qmp_args = dict()
- for k in args.keys():
+ for key in args.keys():
if conv_keys:
- qmp_args[k.translate(self.underscore_to_dash)] = args[k]
+ qmp_args[key.translate(self.underscore_to_dash)] = args[key]
else:
- qmp_args[k] = args[k]
+ qmp_args[key] = args[key]
return self._qmp.cmd(cmd, args=qmp_args)
def command(self, cmd, conv_keys=True, **args):
+ '''
+ Invoke a QMP command.
+ On success return the response dict.
+ On failure raise an exception.
+ '''
reply = self.qmp(cmd, conv_keys, **args)
if reply is None:
raise Exception("Monitor is closed")
@@ -193,7 +218,15 @@ class QEMUMachine(object):
return events
def event_wait(self, name, timeout=60.0, match=None):
- # Test if 'match' is a recursive subset of 'event'
+ '''
+ Wait for specified timeout on named event in QMP; optionally filter
+ results by match.
+
+ The 'match' is checked to be a recursive subset of the 'event'; skips
+ branch processing on match's value None
+ {"foo": {"bar": 1}} matches {"foo": None}
+ {"foo": {"bar": 1}} does not matches {"foo": {"baz": None}}
+ '''
def event_match(event, match=None):
if match is None:
return True
@@ -226,4 +259,8 @@ class QEMUMachine(object):
return None
def get_log(self):
+ '''
+ After self.shutdown or failed qemu execution, this returns the output
+ of the qemu process.
+ '''
return self._iolog
--
2.9.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH v6 01/10] qemu.py: Pylint/style fixes
2017-08-18 14:26 ` [Qemu-devel] [PATCH v6 01/10] " Lukáš Doktor
@ 2017-08-24 22:38 ` Cleber Rosa
0 siblings, 0 replies; 25+ messages in thread
From: Cleber Rosa @ 2017-08-24 22:38 UTC (permalink / raw)
To: Lukáš Doktor, qemu-devel
Cc: famz, ehabkost, apahim, armbru, mreitz, jsnow, f4bug
[-- Attachment #1: Type: text/plain, Size: 7690 bytes --]
On 08/18/2017 10:26 AM, Lukáš Doktor wrote:
> No actual code changes, just several pylint/style fixes and docstring
> clarifications.
>
> Signed-off-by: Lukáš Doktor <ldoktor@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> scripts/qemu.py | 73 +++++++++++++++++++++++++++++++++++++++++++--------------
> 1 file changed, 55 insertions(+), 18 deletions(-)
>
> diff --git a/scripts/qemu.py b/scripts/qemu.py
> index 880e3e8..dd679f1 100644
> --- a/scripts/qemu.py
> +++ b/scripts/qemu.py
> @@ -23,8 +23,22 @@ import qmp.qmp
> class QEMUMachine(object):
> '''A QEMU VM'''
>
> - def __init__(self, binary, args=[], wrapper=[], name=None, test_dir="/var/tmp",
> - monitor_address=None, socket_scm_helper=None, debug=False):
> + def __init__(self, binary, args=[], wrapper=[], name=None,
> + test_dir="/var/tmp", monitor_address=None,
> + socket_scm_helper=None, debug=False):
> + '''
> + Initialize a QEMUMachine
> +
> + @param binary: path to the qemu binary
> + @param args: list of extra arguments
> + @param wrapper: list of arguments used as prefix to qemu binary
> + @param name: prefix for socket and log file names (default: qemu-PID)
> + @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 name is None:
> name = "qemu-%d" % os.getpid()
> if monitor_address is None:
> @@ -33,12 +47,13 @@ class QEMUMachine(object):
> self._qemu_log_path = os.path.join(test_dir, name + ".log")
> self._popen = None
> self._binary = binary
> - self._args = list(args) # Force copy args in case we modify them
> + self._args = list(args) # Force copy args in case we modify them
> self._wrapper = wrapper
> self._events = []
> self._iolog = None
> self._socket_scm_helper = socket_scm_helper
> self._debug = debug
> + self._qmp = None
>
> # This can be used to add an unused monitor instance.
> def add_monitor_telnet(self, ip, port):
> @@ -64,16 +79,16 @@ class QEMUMachine(object):
> if self._socket_scm_helper is None:
> print >>sys.stderr, "No path to socket_scm_helper set"
> return -1
> - if os.path.exists(self._socket_scm_helper) == False:
> + if not os.path.exists(self._socket_scm_helper):
> print >>sys.stderr, "%s does not exist" % self._socket_scm_helper
> return -1
> fd_param = ["%s" % self._socket_scm_helper,
> "%d" % self._qmp.get_sock_fd(),
> "%s" % fd_file_path]
> devnull = open('/dev/null', 'rb')
> - p = subprocess.Popen(fd_param, stdin=devnull, stdout=sys.stdout,
> - stderr=sys.stderr)
> - return p.wait()
> + proc = subprocess.Popen(fd_param, stdin=devnull, stdout=sys.stdout,
> + stderr=sys.stderr)
> + return proc.wait()
>
> @staticmethod
> def _remove_if_exists(path):
> @@ -99,8 +114,8 @@ class QEMUMachine(object):
> return self._popen.pid
>
> def _load_io_log(self):
> - with open(self._qemu_log_path, "r") as fh:
> - self._iolog = fh.read()
> + with open(self._qemu_log_path, "r") as iolog:
> + self._iolog = iolog.read()
>
> def _base_args(self):
> if isinstance(self._monitor_address, tuple):
> @@ -114,7 +129,8 @@ class QEMUMachine(object):
> '-display', 'none', '-vga', 'none']
>
> def _pre_launch(self):
> - self._qmp = qmp.qmp.QEMUMonitorProtocol(self._monitor_address, server=True,
> + self._qmp = qmp.qmp.QEMUMonitorProtocol(self._monitor_address,
> + server=True,
> debug=self._debug)
>
> def _post_launch(self):
> @@ -131,9 +147,11 @@ class QEMUMachine(object):
> qemulog = open(self._qemu_log_path, 'wb')
> try:
> self._pre_launch()
> - args = self._wrapper + [self._binary] + self._base_args() + self._args
> + args = (self._wrapper + [self._binary] + self._base_args() +
> + self._args)
> self._popen = subprocess.Popen(args, stdin=devnull, stdout=qemulog,
> - stderr=subprocess.STDOUT, shell=False)
> + stderr=subprocess.STDOUT,
> + shell=False)
> self._post_launch()
> except:
> if self.is_running():
> @@ -154,23 +172,30 @@ class QEMUMachine(object):
>
> exitcode = self._popen.wait()
> if exitcode < 0:
> - sys.stderr.write('qemu received signal %i: %s\n' % (-exitcode, ' '.join(self._args)))
> + sys.stderr.write('qemu received signal %i: %s\n'
> + % (-exitcode, ' '.join(self._args)))
> self._load_io_log()
> self._post_shutdown()
>
> underscore_to_dash = string.maketrans('_', '-')
> +
> def qmp(self, cmd, conv_keys=True, **args):
> - '''Invoke a QMP command and return the result dict'''
> + '''Invoke a QMP command and return the response dict'''
> qmp_args = dict()
> - for k in args.keys():
> + for key in args.keys():
> if conv_keys:
> - qmp_args[k.translate(self.underscore_to_dash)] = args[k]
> + qmp_args[key.translate(self.underscore_to_dash)] = args[key]
> else:
> - qmp_args[k] = args[k]
> + qmp_args[key] = args[key]
>
> return self._qmp.cmd(cmd, args=qmp_args)
>
> def command(self, cmd, conv_keys=True, **args):
> + '''
> + Invoke a QMP command.
> + On success return the response dict.
> + On failure raise an exception.
> + '''
> reply = self.qmp(cmd, conv_keys, **args)
> if reply is None:
> raise Exception("Monitor is closed")
> @@ -193,7 +218,15 @@ class QEMUMachine(object):
> return events
>
> def event_wait(self, name, timeout=60.0, match=None):
> - # Test if 'match' is a recursive subset of 'event'
> + '''
> + Wait for specified timeout on named event in QMP; optionally filter
> + results by match.
> +
> + The 'match' is checked to be a recursive subset of the 'event'; skips
> + branch processing on match's value None
> + {"foo": {"bar": 1}} matches {"foo": None}
> + {"foo": {"bar": 1}} does not matches {"foo": {"baz": None}}
> + '''
> def event_match(event, match=None):
> if match is None:
> return True
> @@ -226,4 +259,8 @@ class QEMUMachine(object):
> return None
>
> def get_log(self):
> + '''
> + After self.shutdown or failed qemu execution, this returns the output
> + of the qemu process.
> + '''
> return self._iolog
>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH v6 02/10] qemu|qtest: Avoid dangerous arguments
2017-08-18 14:26 [Qemu-devel] [PATCH v6 00/10] qemu.py: Pylint/style fixes Lukáš Doktor
2017-08-18 14:26 ` [Qemu-devel] [PATCH v6 01/10] " Lukáš Doktor
@ 2017-08-18 14:26 ` Lukáš Doktor
2017-08-24 22:55 ` Cleber Rosa
2017-08-18 14:26 ` [Qemu-devel] [PATCH v6 03/10] qemu.py: Use iteritems rather than keys() Lukáš Doktor
` (9 subsequent siblings)
11 siblings, 1 reply; 25+ messages in thread
From: Lukáš Doktor @ 2017-08-18 14:26 UTC (permalink / raw)
To: qemu-devel; +Cc: ldoktor, famz, ehabkost, apahim, armbru, mreitz, jsnow, f4bug
The list object is mutable in python and potentially might modify other
object's arguments when used as default argument. Reproducer:
>>> vm1 = QEMUMachine("qemu")
>>> vm2 = QEMUMachine("qemu")
>>> vm1._wrapper.append("foo")
>>> print vm2._wrapper
['foo']
In this case the `args` is actually copied so it would be safe to keep
it, but it's not a good practice to keep it. The same issue applies in
inherited qtest module.
Signed-off-by: Lukáš Doktor <ldoktor@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
---
scripts/qemu.py | 6 +++++-
scripts/qtest.py | 2 +-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/scripts/qemu.py b/scripts/qemu.py
index dd679f1..5d09de4 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -23,7 +23,7 @@ import qmp.qmp
class QEMUMachine(object):
'''A QEMU VM'''
- def __init__(self, binary, args=[], wrapper=[], name=None,
+ def __init__(self, binary, args=None, wrapper=None, name=None,
test_dir="/var/tmp", monitor_address=None,
socket_scm_helper=None, debug=False):
'''
@@ -39,6 +39,10 @@ class QEMUMachine(object):
@param debug: enable debug mode
@note: Qemu process is not started until launch() is used.
'''
+ if args is None:
+ args = []
+ if wrapper is None:
+ wrapper = []
if name is None:
name = "qemu-%d" % os.getpid()
if monitor_address is None:
diff --git a/scripts/qtest.py b/scripts/qtest.py
index d5aecb5..ab183c0 100644
--- a/scripts/qtest.py
+++ b/scripts/qtest.py
@@ -79,7 +79,7 @@ class QEMUQtestProtocol(object):
class QEMUQtestMachine(qemu.QEMUMachine):
'''A QEMU VM'''
- def __init__(self, binary, args=[], name=None, test_dir="/var/tmp",
+ def __init__(self, binary, args=None, name=None, test_dir="/var/tmp",
socket_scm_helper=None):
if name is None:
name = "qemu-%d" % os.getpid()
--
2.9.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH v6 02/10] qemu|qtest: Avoid dangerous arguments
2017-08-18 14:26 ` [Qemu-devel] [PATCH v6 02/10] qemu|qtest: Avoid dangerous arguments Lukáš Doktor
@ 2017-08-24 22:55 ` Cleber Rosa
0 siblings, 0 replies; 25+ messages in thread
From: Cleber Rosa @ 2017-08-24 22:55 UTC (permalink / raw)
To: Lukáš Doktor, qemu-devel
Cc: famz, ehabkost, apahim, armbru, mreitz, jsnow, f4bug
[-- Attachment #1: Type: text/plain, Size: 2325 bytes --]
On 08/18/2017 10:26 AM, Lukáš Doktor wrote:
> The list object is mutable in python and potentially might modify other
> object's arguments when used as default argument. Reproducer:
>
> >>> vm1 = QEMUMachine("qemu")
> >>> vm2 = QEMUMachine("qemu")
> >>> vm1._wrapper.append("foo")
> >>> print vm2._wrapper
> ['foo']
>
> In this case the `args` is actually copied so it would be safe to keep
> it, but it's not a good practice to keep it. The same issue applies in
> inherited qtest module.
>
> Signed-off-by: Lukáš Doktor <ldoktor@redhat.com>
> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> Reviewed-by: John Snow <jsnow@redhat.com>
> ---
> scripts/qemu.py | 6 +++++-
> scripts/qtest.py | 2 +-
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/qemu.py b/scripts/qemu.py
> index dd679f1..5d09de4 100644
> --- a/scripts/qemu.py
> +++ b/scripts/qemu.py
> @@ -23,7 +23,7 @@ import qmp.qmp
> class QEMUMachine(object):
> '''A QEMU VM'''
>
> - def __init__(self, binary, args=[], wrapper=[], name=None,
> + def __init__(self, binary, args=None, wrapper=None, name=None,
> test_dir="/var/tmp", monitor_address=None,
> socket_scm_helper=None, debug=False):
> '''
> @@ -39,6 +39,10 @@ class QEMUMachine(object):
> @param debug: enable debug mode
> @note: Qemu process is not started until launch() is used.
> '''
> + if args is None:
> + args = []
> + if wrapper is None:
> + wrapper = []
> if name is None:
> name = "qemu-%d" % os.getpid()
> if monitor_address is None:
> diff --git a/scripts/qtest.py b/scripts/qtest.py
> index d5aecb5..ab183c0 100644
> --- a/scripts/qtest.py
> +++ b/scripts/qtest.py
> @@ -79,7 +79,7 @@ class QEMUQtestProtocol(object):
> class QEMUQtestMachine(qemu.QEMUMachine):
> '''A QEMU VM'''
>
> - def __init__(self, binary, args=[], name=None, test_dir="/var/tmp",
> + def __init__(self, binary, args=None, name=None, test_dir="/var/tmp",
> socket_scm_helper=None):
> if name is None:
> name = "qemu-%d" % os.getpid()
>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH v6 03/10] qemu.py: Use iteritems rather than keys()
2017-08-18 14:26 [Qemu-devel] [PATCH v6 00/10] qemu.py: Pylint/style fixes Lukáš Doktor
2017-08-18 14:26 ` [Qemu-devel] [PATCH v6 01/10] " Lukáš Doktor
2017-08-18 14:26 ` [Qemu-devel] [PATCH v6 02/10] qemu|qtest: Avoid dangerous arguments Lukáš Doktor
@ 2017-08-18 14:26 ` Lukáš Doktor
2017-08-24 22:56 ` Cleber Rosa
2017-08-18 14:26 ` [Qemu-devel] [PATCH v6 04/10] qemu.py: Simplify QMP key-conversion Lukáš Doktor
` (8 subsequent siblings)
11 siblings, 1 reply; 25+ messages in thread
From: Lukáš Doktor @ 2017-08-18 14:26 UTC (permalink / raw)
To: qemu-devel; +Cc: ldoktor, famz, ehabkost, apahim, armbru, mreitz, jsnow, f4bug
Let's avoid creating an in-memory list of keys and query for each value
and use `iteritems` which is an iterator of key-value pairs.
Signed-off-by: Lukáš Doktor <ldoktor@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
scripts/qemu.py | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/scripts/qemu.py b/scripts/qemu.py
index 5d09de4..db21407 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -186,11 +186,11 @@ class QEMUMachine(object):
def qmp(self, cmd, conv_keys=True, **args):
'''Invoke a QMP command and return the response dict'''
qmp_args = dict()
- for key in args.keys():
+ for key, value in args.iteritems():
if conv_keys:
- qmp_args[key.translate(self.underscore_to_dash)] = args[key]
+ qmp_args[key.translate(self.underscore_to_dash)] = value
else:
- qmp_args[key] = args[key]
+ qmp_args[key] = value
return self._qmp.cmd(cmd, args=qmp_args)
--
2.9.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH v6 03/10] qemu.py: Use iteritems rather than keys()
2017-08-18 14:26 ` [Qemu-devel] [PATCH v6 03/10] qemu.py: Use iteritems rather than keys() Lukáš Doktor
@ 2017-08-24 22:56 ` Cleber Rosa
0 siblings, 0 replies; 25+ messages in thread
From: Cleber Rosa @ 2017-08-24 22:56 UTC (permalink / raw)
To: Lukáš Doktor, qemu-devel
Cc: famz, ehabkost, apahim, armbru, mreitz, jsnow, f4bug
[-- Attachment #1: Type: text/plain, Size: 1348 bytes --]
On 08/18/2017 10:26 AM, Lukáš Doktor wrote:
> Let's avoid creating an in-memory list of keys and query for each value
> and use `iteritems` which is an iterator of key-value pairs.
>
> Signed-off-by: Lukáš Doktor <ldoktor@redhat.com>
> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> scripts/qemu.py | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/qemu.py b/scripts/qemu.py
> index 5d09de4..db21407 100644
> --- a/scripts/qemu.py
> +++ b/scripts/qemu.py
> @@ -186,11 +186,11 @@ class QEMUMachine(object):
> def qmp(self, cmd, conv_keys=True, **args):
> '''Invoke a QMP command and return the response dict'''
> qmp_args = dict()
> - for key in args.keys():
> + for key, value in args.iteritems():
> if conv_keys:
> - qmp_args[key.translate(self.underscore_to_dash)] = args[key]
> + qmp_args[key.translate(self.underscore_to_dash)] = value
> else:
> - qmp_args[key] = args[key]
> + qmp_args[key] = value
>
> return self._qmp.cmd(cmd, args=qmp_args)
>
>
Yep, I could tell this was coming while reading patch 01.
Reviewed-by: Cleber Rosa <crosa@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH v6 04/10] qemu.py: Simplify QMP key-conversion
2017-08-18 14:26 [Qemu-devel] [PATCH v6 00/10] qemu.py: Pylint/style fixes Lukáš Doktor
` (2 preceding siblings ...)
2017-08-18 14:26 ` [Qemu-devel] [PATCH v6 03/10] qemu.py: Use iteritems rather than keys() Lukáš Doktor
@ 2017-08-18 14:26 ` Lukáš Doktor
2017-08-24 22:59 ` Cleber Rosa
2017-08-18 14:26 ` [Qemu-devel] [PATCH v6 05/10] qemu.py: Use custom exceptions rather than Exception Lukáš Doktor
` (7 subsequent siblings)
11 siblings, 1 reply; 25+ messages in thread
From: Lukáš Doktor @ 2017-08-18 14:26 UTC (permalink / raw)
To: qemu-devel; +Cc: ldoktor, famz, ehabkost, apahim, armbru, mreitz, jsnow, f4bug
The QMP key conversion consist of '_'s to be replaced with '-'s, which
can easily be done by a single `str.replace` method which is faster and
does not require `string` module import.
Signed-off-by: Lukáš Doktor <ldoktor@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
---
scripts/qemu.py | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/scripts/qemu.py b/scripts/qemu.py
index db21407..19cbd34 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -13,7 +13,6 @@
#
import errno
-import string
import os
import sys
import subprocess
@@ -181,14 +180,12 @@ class QEMUMachine(object):
self._load_io_log()
self._post_shutdown()
- underscore_to_dash = string.maketrans('_', '-')
-
def qmp(self, cmd, conv_keys=True, **args):
'''Invoke a QMP command and return the response dict'''
qmp_args = dict()
for key, value in args.iteritems():
if conv_keys:
- qmp_args[key.translate(self.underscore_to_dash)] = value
+ qmp_args[key.replace('_', '-')] = value
else:
qmp_args[key] = value
--
2.9.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH v6 04/10] qemu.py: Simplify QMP key-conversion
2017-08-18 14:26 ` [Qemu-devel] [PATCH v6 04/10] qemu.py: Simplify QMP key-conversion Lukáš Doktor
@ 2017-08-24 22:59 ` Cleber Rosa
0 siblings, 0 replies; 25+ messages in thread
From: Cleber Rosa @ 2017-08-24 22:59 UTC (permalink / raw)
To: Lukáš Doktor, qemu-devel
Cc: famz, ehabkost, apahim, armbru, mreitz, jsnow, f4bug
[-- Attachment #1: Type: text/plain, Size: 1470 bytes --]
On 08/18/2017 10:26 AM, Lukáš Doktor wrote:
> The QMP key conversion consist of '_'s to be replaced with '-'s, which
> can easily be done by a single `str.replace` method which is faster and
> does not require `string` module import.
>
> Signed-off-by: Lukáš Doktor <ldoktor@redhat.com>
> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> scripts/qemu.py | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/scripts/qemu.py b/scripts/qemu.py
> index db21407..19cbd34 100644
> --- a/scripts/qemu.py
> +++ b/scripts/qemu.py
> @@ -13,7 +13,6 @@
> #
>
> import errno
> -import string
> import os
> import sys
> import subprocess
> @@ -181,14 +180,12 @@ class QEMUMachine(object):
> self._load_io_log()
> self._post_shutdown()
>
> - underscore_to_dash = string.maketrans('_', '-')
> -
> def qmp(self, cmd, conv_keys=True, **args):
> '''Invoke a QMP command and return the response dict'''
> qmp_args = dict()
> for key, value in args.iteritems():
> if conv_keys:
> - qmp_args[key.translate(self.underscore_to_dash)] = value
> + qmp_args[key.replace('_', '-')] = value
> else:
> qmp_args[key] = value
>
>
Right, this would only make sense if multiple (different) substitutions
were necessary.
Reviewed-by: Cleber Rosa <crosa@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH v6 05/10] qemu.py: Use custom exceptions rather than Exception
2017-08-18 14:26 [Qemu-devel] [PATCH v6 00/10] qemu.py: Pylint/style fixes Lukáš Doktor
` (3 preceding siblings ...)
2017-08-18 14:26 ` [Qemu-devel] [PATCH v6 04/10] qemu.py: Simplify QMP key-conversion Lukáš Doktor
@ 2017-08-18 14:26 ` Lukáš Doktor
2017-08-18 14:26 ` [Qemu-devel] [PATCH v6 06/10] qmp.py: Couple of pylint/style fixes Lukáš Doktor
` (6 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Lukáš Doktor @ 2017-08-18 14:26 UTC (permalink / raw)
To: qemu-devel; +Cc: ldoktor, famz, ehabkost, apahim, armbru, mreitz, jsnow, f4bug
The naked Exception should not be widely used. It makes sense to be a
bit more specific and use better-suited custom exceptions. As a benefit
we can store the full reply in the exception in case someone needs it
when catching the exception.
Signed-off-by: Lukáš Doktor <ldoktor@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
---
scripts/qemu.py | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/scripts/qemu.py b/scripts/qemu.py
index 19cbd34..97ecb63 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -19,6 +19,19 @@ import subprocess
import qmp.qmp
+class MonitorResponseError(qmp.qmp.QMPError):
+ '''
+ Represents erroneous QMP monitor reply
+ '''
+ def __init__(self, reply):
+ try:
+ desc = reply["error"]["desc"]
+ except KeyError:
+ desc = reply
+ super(MonitorResponseError, self).__init__(desc)
+ self.reply = reply
+
+
class QEMUMachine(object):
'''A QEMU VM'''
@@ -199,9 +212,9 @@ class QEMUMachine(object):
'''
reply = self.qmp(cmd, conv_keys, **args)
if reply is None:
- raise Exception("Monitor is closed")
+ raise qmp.qmp.QMPError("Monitor is closed")
if "error" in reply:
- raise Exception(reply["error"]["desc"])
+ raise MonitorResponseError(reply)
return reply["return"]
def get_qmp_event(self, wait=False):
--
2.9.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PATCH v6 06/10] qmp.py: Couple of pylint/style fixes
2017-08-18 14:26 [Qemu-devel] [PATCH v6 00/10] qemu.py: Pylint/style fixes Lukáš Doktor
` (4 preceding siblings ...)
2017-08-18 14:26 ` [Qemu-devel] [PATCH v6 05/10] qemu.py: Use custom exceptions rather than Exception Lukáš Doktor
@ 2017-08-18 14:26 ` Lukáš Doktor
2017-08-18 14:26 ` [Qemu-devel] [PATCH v6 07/10] qmp.py: Use object-based class for QEMUMonitorProtocol Lukáš Doktor
` (5 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Lukáš Doktor @ 2017-08-18 14:26 UTC (permalink / raw)
To: qemu-devel; +Cc: ldoktor, famz, ehabkost, apahim, armbru, mreitz, jsnow, f4bug
No actual code changes, just initializing attributes earlier to avoid
AttributeError on early introspection, a few pylint/style fixes and
docstring clarifications.
Signed-off-by: Lukáš Doktor <ldoktor@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
scripts/qmp/qmp.py | 37 ++++++++++++++++++++++++-------------
1 file changed, 24 insertions(+), 13 deletions(-)
diff --git a/scripts/qmp/qmp.py b/scripts/qmp/qmp.py
index 62d3651..782d1ac 100644
--- a/scripts/qmp/qmp.py
+++ b/scripts/qmp/qmp.py
@@ -13,19 +13,30 @@ import errno
import socket
import sys
+
class QMPError(Exception):
pass
+
class QMPConnectError(QMPError):
pass
+
class QMPCapabilitiesError(QMPError):
pass
+
class QMPTimeoutError(QMPError):
pass
+
class QEMUMonitorProtocol:
+
+ #: Socket's error class
+ error = socket.error
+ #: Socket's timeout
+ timeout = socket.timeout
+
def __init__(self, address, server=False, debug=False):
"""
Create a QEMUMonitorProtocol class.
@@ -42,6 +53,7 @@ class QEMUMonitorProtocol:
self.__address = address
self._debug = debug
self.__sock = self.__get_sock()
+ self.__sockfile = None
if server:
self.__sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
self.__sock.bind(self.__address)
@@ -56,7 +68,7 @@ class QEMUMonitorProtocol:
def __negotiate_capabilities(self):
greeting = self.__json_read()
- if greeting is None or not greeting.has_key('QMP'):
+ if greeting is None or "QMP" not in greeting:
raise QMPConnectError
# Greeting seems ok, negotiate capabilities
resp = self.cmd('qmp_capabilities')
@@ -78,8 +90,6 @@ class QEMUMonitorProtocol:
continue
return resp
- error = socket.error
-
def __get_events(self, wait=False):
"""
Check for new events in the stream and cache them in __events.
@@ -89,8 +99,8 @@ class QEMUMonitorProtocol:
@raise QMPTimeoutError: If a timeout float is provided and the timeout
period elapses.
- @raise QMPConnectError: If wait is True but no events could be retrieved
- or if some other error occurred.
+ @raise QMPConnectError: If wait is True but no events could be
+ retrieved or if some other error occurred.
"""
# Check for new events regardless and pull them into the cache:
@@ -175,7 +185,7 @@ class QEMUMonitorProtocol:
@param args: command arguments (dict)
@param id: command id (dict, list, string or int)
"""
- qmp_cmd = { 'execute': name }
+ qmp_cmd = {'execute': name}
if args:
qmp_cmd['arguments'] = args
if id:
@@ -183,6 +193,9 @@ class QEMUMonitorProtocol:
return self.cmd_obj(qmp_cmd)
def command(self, cmd, **kwds):
+ """
+ Build and send a QMP command to the monitor, report errors if any
+ """
ret = self.cmd(cmd, kwds)
if ret.has_key('error'):
raise Exception(ret['error']['desc'])
@@ -190,15 +203,15 @@ class QEMUMonitorProtocol:
def pull_event(self, wait=False):
"""
- Get and delete the first available QMP event.
+ Pulls a single event.
@param wait (bool): block until an event is available.
@param wait (float): If wait is a float, treat it as a timeout value.
@raise QMPTimeoutError: If a timeout float is provided and the timeout
period elapses.
- @raise QMPConnectError: If wait is True but no events could be retrieved
- or if some other error occurred.
+ @raise QMPConnectError: If wait is True but no events could be
+ retrieved or if some other error occurred.
@return The first available QMP event, or None.
"""
@@ -217,8 +230,8 @@ class QEMUMonitorProtocol:
@raise QMPTimeoutError: If a timeout float is provided and the timeout
period elapses.
- @raise QMPConnectError: If wait is True but no events could be retrieved
- or if some other error occurred.
+ @raise QMPConnectError: If wait is True but no events could be
+ retrieved or if some other error occurred.
@return The list of available QMP events.
"""
@@ -235,8 +248,6 @@ class QEMUMonitorProtocol:
self.__sock.close()
self.__sockfile.close()
- timeout = socket.timeout
-
def settimeout(self, timeout):
self.__sock.settimeout(timeout)
--
2.9.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PATCH v6 07/10] qmp.py: Use object-based class for QEMUMonitorProtocol
2017-08-18 14:26 [Qemu-devel] [PATCH v6 00/10] qemu.py: Pylint/style fixes Lukáš Doktor
` (5 preceding siblings ...)
2017-08-18 14:26 ` [Qemu-devel] [PATCH v6 06/10] qmp.py: Couple of pylint/style fixes Lukáš Doktor
@ 2017-08-18 14:26 ` Lukáš Doktor
2017-08-18 14:26 ` [Qemu-devel] [PATCH v6 08/10] qmp.py: Avoid "has_key" usage Lukáš Doktor
` (4 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Lukáš Doktor @ 2017-08-18 14:26 UTC (permalink / raw)
To: qemu-devel; +Cc: ldoktor, famz, ehabkost, apahim, armbru, mreitz, jsnow, f4bug
There is no need to define QEMUMonitorProtocol as old-style class.
Signed-off-by: Lukáš Doktor <ldoktor@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
---
scripts/qmp/qmp-shell | 4 ++--
scripts/qmp/qmp.py | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 860ffb2..be449de 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -106,7 +106,7 @@ class FuzzyJSON(ast.NodeTransformer):
# _execute_cmd()). Let's design a better one.
class QMPShell(qmp.QEMUMonitorProtocol):
def __init__(self, address, pretty=False):
- qmp.QEMUMonitorProtocol.__init__(self, self.__get_address(address))
+ super(QMPShell, self).__init__(self.__get_address(address))
self._greeting = None
self._completer = None
self._pretty = pretty
@@ -281,7 +281,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
return True
def connect(self, negotiate):
- self._greeting = qmp.QEMUMonitorProtocol.connect(self, negotiate)
+ self._greeting = super(QMPShell, self).connect(negotiate)
self.__completer_setup()
def show_banner(self, msg='Welcome to the QMP low-level shell!'):
diff --git a/scripts/qmp/qmp.py b/scripts/qmp/qmp.py
index 782d1ac..95ff5cc 100644
--- a/scripts/qmp/qmp.py
+++ b/scripts/qmp/qmp.py
@@ -30,7 +30,7 @@ class QMPTimeoutError(QMPError):
pass
-class QEMUMonitorProtocol:
+class QEMUMonitorProtocol(object):
#: Socket's error class
error = socket.error
--
2.9.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PATCH v6 08/10] qmp.py: Avoid "has_key" usage
2017-08-18 14:26 [Qemu-devel] [PATCH v6 00/10] qemu.py: Pylint/style fixes Lukáš Doktor
` (6 preceding siblings ...)
2017-08-18 14:26 ` [Qemu-devel] [PATCH v6 07/10] qmp.py: Use object-based class for QEMUMonitorProtocol Lukáš Doktor
@ 2017-08-18 14:26 ` Lukáš Doktor
2017-08-18 14:26 ` [Qemu-devel] [PATCH v6 09/10] qmp.py: Avoid overriding a builtin object Lukáš Doktor
` (3 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Lukáš Doktor @ 2017-08-18 14:26 UTC (permalink / raw)
To: qemu-devel; +Cc: ldoktor, famz, ehabkost, apahim, armbru, mreitz, jsnow, f4bug
The "has_key" is deprecated in favor of "__in__" operator.
Signed-off-by: Lukáš Doktor <ldoktor@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
scripts/qmp/qmp.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/qmp/qmp.py b/scripts/qmp/qmp.py
index 95ff5cc..f2f5a9b 100644
--- a/scripts/qmp/qmp.py
+++ b/scripts/qmp/qmp.py
@@ -197,7 +197,7 @@ class QEMUMonitorProtocol(object):
Build and send a QMP command to the monitor, report errors if any
"""
ret = self.cmd(cmd, kwds)
- if ret.has_key('error'):
+ if "error" in ret:
raise Exception(ret['error']['desc'])
return ret['return']
--
2.9.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PATCH v6 09/10] qmp.py: Avoid overriding a builtin object
2017-08-18 14:26 [Qemu-devel] [PATCH v6 00/10] qemu.py: Pylint/style fixes Lukáš Doktor
` (7 preceding siblings ...)
2017-08-18 14:26 ` [Qemu-devel] [PATCH v6 08/10] qmp.py: Avoid "has_key" usage Lukáš Doktor
@ 2017-08-18 14:26 ` Lukáš Doktor
2017-08-18 14:26 ` [Qemu-devel] [PATCH v6 10/10] qtest.py: Few pylint/style fixes Lukáš Doktor
` (2 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Lukáš Doktor @ 2017-08-18 14:26 UTC (permalink / raw)
To: qemu-devel; +Cc: ldoktor, famz, ehabkost, apahim, armbru, mreitz, jsnow, f4bug
The "id" is a builtin method to get object's identity and should not be
overridden. This might bring some issues in case someone was directly
calling "cmd(..., id=id)" but I haven't found such usage on brief search
for "cmd\(.*id=".
Signed-off-by: Lukáš Doktor <ldoktor@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
---
scripts/qmp/qmp.py | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/scripts/qmp/qmp.py b/scripts/qmp/qmp.py
index f2f5a9b..ef12e8a 100644
--- a/scripts/qmp/qmp.py
+++ b/scripts/qmp/qmp.py
@@ -177,19 +177,19 @@ class QEMUMonitorProtocol(object):
print >>sys.stderr, "QMP:<<< %s" % resp
return resp
- def cmd(self, name, args=None, id=None):
+ def cmd(self, name, args=None, cmd_id=None):
"""
Build a QMP command and send it to the QMP Monitor.
@param name: command name (string)
@param args: command arguments (dict)
- @param id: command id (dict, list, string or int)
+ @param cmd_id: command id (dict, list, string or int)
"""
qmp_cmd = {'execute': name}
if args:
qmp_cmd['arguments'] = args
- if id:
- qmp_cmd['id'] = id
+ if cmd_id:
+ qmp_cmd['id'] = cmd_id
return self.cmd_obj(qmp_cmd)
def command(self, cmd, **kwds):
--
2.9.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PATCH v6 10/10] qtest.py: Few pylint/style fixes
2017-08-18 14:26 [Qemu-devel] [PATCH v6 00/10] qemu.py: Pylint/style fixes Lukáš Doktor
` (8 preceding siblings ...)
2017-08-18 14:26 ` [Qemu-devel] [PATCH v6 09/10] qmp.py: Avoid overriding a builtin object Lukáš Doktor
@ 2017-08-18 14:26 ` Lukáš Doktor
2017-08-21 21:51 ` [Qemu-devel] [PATCH v6 00/10] qemu.py: Pylint/style fixes John Snow
2017-08-30 21:18 ` Eduardo Habkost
11 siblings, 0 replies; 25+ messages in thread
From: Lukáš Doktor @ 2017-08-18 14:26 UTC (permalink / raw)
To: qemu-devel; +Cc: ldoktor, famz, ehabkost, apahim, armbru, mreitz, jsnow, f4bug
No actual code changes, just few pylint/style fixes.
Signed-off-by: Lukáš Doktor <ldoktor@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
---
scripts/qtest.py | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/scripts/qtest.py b/scripts/qtest.py
index ab183c0..df0daf2 100644
--- a/scripts/qtest.py
+++ b/scripts/qtest.py
@@ -11,14 +11,11 @@
# Based on qmp.py.
#
-import errno
import socket
-import string
import os
-import subprocess
-import qmp.qmp
import qemu
+
class QEMUQtestProtocol(object):
def __init__(self, address, server=False):
"""
@@ -83,8 +80,10 @@ class QEMUQtestMachine(qemu.QEMUMachine):
socket_scm_helper=None):
if name is None:
name = "qemu-%d" % os.getpid()
- super(QEMUQtestMachine, self).__init__(binary, args, name=name, test_dir=test_dir,
- socket_scm_helper=socket_scm_helper)
+ super(QEMUQtestMachine,
+ self).__init__(binary, args, name=name, test_dir=test_dir,
+ socket_scm_helper=socket_scm_helper)
+ self._qtest = None
self._qtest_path = os.path.join(test_dir, name + "-qtest.sock")
def _base_args(self):
--
2.9.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH v6 00/10] qemu.py: Pylint/style fixes
2017-08-18 14:26 [Qemu-devel] [PATCH v6 00/10] qemu.py: Pylint/style fixes Lukáš Doktor
` (9 preceding siblings ...)
2017-08-18 14:26 ` [Qemu-devel] [PATCH v6 10/10] qtest.py: Few pylint/style fixes Lukáš Doktor
@ 2017-08-21 21:51 ` John Snow
2017-08-22 7:24 ` Markus Armbruster
2017-08-30 21:18 ` Eduardo Habkost
11 siblings, 1 reply; 25+ messages in thread
From: John Snow @ 2017-08-21 21:51 UTC (permalink / raw)
To: Lukáš Doktor, qemu-devel
Cc: famz, ehabkost, apahim, armbru, mreitz, f4bug
On 08/18/2017 10:26 AM, Lukáš Doktor wrote:
> Hello guys,
>
> I'm reading the available python modules to exercise qemu and while reading them
> I fixed some issues that caught my attention. It usually starts with a simple
> pylint/docstring fixes and slowly graduates to more controversial ones so I'm
> open to suggestion to remove some of them.
>
> Kind regards,
> Lukáš
>
> Changes in v2
> - Squashed 2nd and 10th patches into 2nd one
> - Use repr() in MonitorResponseError's description
> - Improved commit message of the 6th patch
> - Two tweaks to docstrings changed in the 6th patch
> - Also updated qmp-shell to use new-style super calls (7th patch)
> - Fixed the accidental rename of qmp `cmd_id` (kept the id->cmd_id patch)
> - Changed the style of the style-fix in the 10th commit
>
> Changes in v3
> - Don't use repr in the 5th patch in MonitorResponseError
>
> Changes in v4
> - Use correct git base (remove unwanted commits)
>
> Changes in v5
> - Avoid bool comparison
> - Change report to return in one docstring
> - Removed the unnecessary spaces around single-line docstring
>
> Changes in v6
> - Bunch of docstring tweaks by Markus Armbruster
> - Line break in <80 chars
> - result dict => response dict
> - Removed the "event_match" rename
>
Looks like all ten patches have an R-B despite changes; but it looks
like nothing particularly major was changed anyway.
Does this fall under Markus's jurisdiction?
(Well, except for qtest.py which seemingly has double-extra-no
maintainer...!)
--js
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH v6 00/10] qemu.py: Pylint/style fixes
2017-08-21 21:51 ` [Qemu-devel] [PATCH v6 00/10] qemu.py: Pylint/style fixes John Snow
@ 2017-08-22 7:24 ` Markus Armbruster
2017-08-22 10:19 ` Philippe Mathieu-Daudé
` (3 more replies)
0 siblings, 4 replies; 25+ messages in thread
From: Markus Armbruster @ 2017-08-22 7:24 UTC (permalink / raw)
To: John Snow
Cc: Lukáš Doktor, qemu-devel, famz, ehabkost, apahim, f4bug,
mreitz, Daniel P. Berrange, Kevin Wolf, Paolo Bonzini
John Snow <jsnow@redhat.com> writes:
> On 08/18/2017 10:26 AM, Lukáš Doktor wrote:
>> Hello guys,
>>
>> I'm reading the available python modules to exercise qemu and while reading them
>> I fixed some issues that caught my attention. It usually starts with a simple
>> pylint/docstring fixes and slowly graduates to more controversial ones so I'm
>> open to suggestion to remove some of them.
>>
>> Kind regards,
>> Lukáš
>>
>> Changes in v2
>> - Squashed 2nd and 10th patches into 2nd one
>> - Use repr() in MonitorResponseError's description
>> - Improved commit message of the 6th patch
>> - Two tweaks to docstrings changed in the 6th patch
>> - Also updated qmp-shell to use new-style super calls (7th patch)
>> - Fixed the accidental rename of qmp `cmd_id` (kept the id->cmd_id patch)
>> - Changed the style of the style-fix in the 10th commit
>>
>> Changes in v3
>> - Don't use repr in the 5th patch in MonitorResponseError
>>
>> Changes in v4
>> - Use correct git base (remove unwanted commits)
>>
>> Changes in v5
>> - Avoid bool comparison
>> - Change report to return in one docstring
>> - Removed the unnecessary spaces around single-line docstring
>>
>> Changes in v6
>> - Bunch of docstring tweaks by Markus Armbruster
>> - Line break in <80 chars
>> - result dict => response dict
>> - Removed the "event_match" rename
>>
>
> Looks like all ten patches have an R-B despite changes; but it looks
> like nothing particularly major was changed anyway.
>
> Does this fall under Markus's jurisdiction?
>
> (Well, except for qtest.py which seemingly has double-extra-no
> maintainer...!)
qemu.py is about starting and controlling QEMU, commonly for testing
purposes. It's related to QMP only by virtue of using QMP for control
(well, what else could it use?); if that makes me maintainer, I'll soon
maintain basically all tests :)
As far as I can tell, qemu.py's main user is still qemu-iotests, via
qtest.py. Dan factored it out to make it available for
tests/migration/guestperf/.
Options for maintaining qemu.py and qtest.py:
* Maintain them with qemu-iotest
Currently mainained with the block layer core, by Kevin and Max.
- Keep it that way
- Appoint qemu-iotest maintainer(s).
* Maintain them separately, say as "Python qtest support", appoint
maintainer(s)
Dan appears to be a hot contender:
$ scripts/get_maintainer.pl -f --git-blame scripts/qtest.py
Fam Zheng <famz@redhat.com> (authored lines:71/110=65%,commits:1/3=33%)
"Daniel P. Berrange" <berrange@redhat.com> (authored lines:39/110=35%,commits:2/3=67%)
Max Reitz <mreitz@redhat.com> (commits:2/3=67%)
Amit Shah <amit.shah@redhat.com> (commits:1/3=33%)
Stefan Hajnoczi <stefanha@redhat.com> (commits:1/3=33%)
qemu-devel@nongnu.org (open list:All patches CC here)
$ scripts/get_maintainer.pl -f --git-blame scripts/qemu.py
"Daniel P. Berrange" <berrange@redhat.com> (authored lines:217/229=95%,commits:2/4=50%)
Eduardo Habkost <ehabkost@redhat.com> (authored lines:12/229=5%,commits:4/4=100%)
Markus Armbruster <armbru@redhat.com> (commits:1/4=25%)
Max Reitz <mreitz@redhat.com> (commits:1/4=25%)
Amit Shah <amit.shah@redhat.com> (commits:1/4=25%)
qemu-devel@nongnu.org (open list:All patches CC here)
Eduardo made the mistake^W^W^Wgraciously volunteered to maintain
scripts/qmp/qmp-shell, which is also used for testing. Perhaps he'd
be willing to maintain these guys as well.
* Do nothing
Hope "somebody" will take pity and merge patches. A common value of
"somebody" would be Paolo.
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH v6 00/10] qemu.py: Pylint/style fixes
2017-08-22 7:24 ` Markus Armbruster
@ 2017-08-22 10:19 ` Philippe Mathieu-Daudé
2017-08-22 19:07 ` Eduardo Habkost
2017-08-22 13:56 ` Paolo Bonzini
` (2 subsequent siblings)
3 siblings, 1 reply; 25+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-08-22 10:19 UTC (permalink / raw)
To: Markus Armbruster, John Snow
Cc: Lukáš Doktor, qemu-devel, famz, ehabkost, apahim,
mreitz, Daniel P. Berrange, Kevin Wolf, Paolo Bonzini
On 08/22/2017 04:24 AM, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:
[...]
>>
>> Looks like all ten patches have an R-B despite changes; but it looks
>> like nothing particularly major was changed anyway.
>>
>> Does this fall under Markus's jurisdiction?
>>
>> (Well, except for qtest.py which seemingly has double-extra-no
>> maintainer...!)
>
> qemu.py is about starting and controlling QEMU, commonly for testing
> purposes. It's related to QMP only by virtue of using QMP for control
> (well, what else could it use?); if that makes me maintainer, I'll soon
> maintain basically all tests :)
>
> As far as I can tell, qemu.py's main user is still qemu-iotests, via
> qtest.py. Dan factored it out to make it available for
> tests/migration/guestperf/.
>
> Options for maintaining qemu.py and qtest.py:
>
> * Maintain them with qemu-iotest
>
> Currently mainained with the block layer core, by Kevin and Max.
>
> - Keep it that way
>
> - Appoint qemu-iotest maintainer(s).
>
> * Maintain them separately, say as "Python qtest support", appoint
> maintainer(s)
>
> Dan appears to be a hot contender:
>
> $ scripts/get_maintainer.pl -f --git-blame scripts/qtest.py
> Fam Zheng <famz@redhat.com> (authored lines:71/110=65%,commits:1/3=33%)
> "Daniel P. Berrange" <berrange@redhat.com> (authored lines:39/110=35%,commits:2/3=67%)
> Max Reitz <mreitz@redhat.com> (commits:2/3=67%)
> Amit Shah <amit.shah@redhat.com> (commits:1/3=33%)
> Stefan Hajnoczi <stefanha@redhat.com> (commits:1/3=33%)
> qemu-devel@nongnu.org (open list:All patches CC here)
> $ scripts/get_maintainer.pl -f --git-blame scripts/qemu.py
> "Daniel P. Berrange" <berrange@redhat.com> (authored lines:217/229=95%,commits:2/4=50%)
> Eduardo Habkost <ehabkost@redhat.com> (authored lines:12/229=5%,commits:4/4=100%)
> Markus Armbruster <armbru@redhat.com> (commits:1/4=25%)
> Max Reitz <mreitz@redhat.com> (commits:1/4=25%)
> Amit Shah <amit.shah@redhat.com> (commits:1/4=25%)
> qemu-devel@nongnu.org (open list:All patches CC here)
>
> Eduardo made the mistake^W^W^Wgraciously volunteered to maintain
> scripts/qmp/qmp-shell, which is also used for testing. Perhaps he'd
> be willing to maintain these guys as well.
>
> * Do nothing
>
> Hope "somebody" will take pity and merge patches. A common value of
> "somebody" would be Paolo.
:'(
* Fam is also a good candidate for scripts/qemu.py with VM testing
* trivial queue?
* Add global Python maintainers
Mostly to review/merge...
Can we predict how the python scripts will evolve? Only fast-testing?
Is there some users hacking on qemu.py unaware they can/should use
libvirt-python?
If so, shouldn't we think about running more tests through libvirt and
add effort there?
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PATCH v6 00/10] qemu.py: Pylint/style fixes
2017-08-22 10:19 ` Philippe Mathieu-Daudé
@ 2017-08-22 19:07 ` Eduardo Habkost
2017-08-24 12:15 ` Stefan Hajnoczi
0 siblings, 1 reply; 25+ messages in thread
From: Eduardo Habkost @ 2017-08-22 19:07 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Markus Armbruster, John Snow, Lukáš Doktor, qemu-devel,
famz, apahim, mreitz, Daniel P. Berrange, Kevin Wolf,
Paolo Bonzini, Cleber Rosa, Stefan Hajnoczi
(CCing Cleber and Stefan)
On Tue, Aug 22, 2017 at 07:19:45AM -0300, Philippe Mathieu-Daudé wrote:
[...]
> Can we predict how the python scripts will evolve? Only fast-testing?
>
I guess it depends on how you define "fast". Does "fast-testing"
include a full device-crash-test run (that could take ~30
minutes) or the full set of iotests?
> Is there some users hacking on qemu.py unaware they can/should use
> libvirt-python?
I believe the existing users of qemu.py wouldn't want to have
dependencies on libvirt or other external modules (e.g. code that
test specific QMP commands and/or is run by "make check").
> If so, shouldn't we think about running more tests through libvirt and add
> effort there?
Even if the existing test code wouldn't benefit from libvirt, I
agree there is some value in writing test code that uses libvirt.
But in this case would the test code belong to the QEMU source
tree, or somewhere else?
--
Eduardo
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v6 00/10] qemu.py: Pylint/style fixes
2017-08-22 19:07 ` Eduardo Habkost
@ 2017-08-24 12:15 ` Stefan Hajnoczi
2017-08-24 12:49 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 25+ messages in thread
From: Stefan Hajnoczi @ 2017-08-24 12:15 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Philippe Mathieu-Daudé, Markus Armbruster, John Snow,
Lukáš Doktor, qemu-devel, famz, apahim, mreitz,
Daniel P. Berrange, Kevin Wolf, Paolo Bonzini, Cleber Rosa
On Tue, Aug 22, 2017 at 04:07:09PM -0300, Eduardo Habkost wrote:
> (CCing Cleber and Stefan)
>
> On Tue, Aug 22, 2017 at 07:19:45AM -0300, Philippe Mathieu-Daudé wrote:
> [...]
> > Can we predict how the python scripts will evolve? Only fast-testing?
> >
>
> I guess it depends on how you define "fast". Does "fast-testing"
> include a full device-crash-test run (that could take ~30
> minutes) or the full set of iotests?
>
>
> > Is there some users hacking on qemu.py unaware they can/should use
> > libvirt-python?
>
> I believe the existing users of qemu.py wouldn't want to have
> dependencies on libvirt or other external modules (e.g. code that
> test specific QMP commands and/or is run by "make check").
Yes, most QEMU tests interact with QEMU at a lower level than the
libvirt API. They may exercise new features that are not available via
libvirt so we'd end up bypassing it anyway.
It is easier to debug a test case that interacts directly with QEMU.
Investigating a failed test where libvirt is involved is more
time-consuming.
libvirt and tools built on it, like virt-builder, are good for tests
that need a fully-fledged operating system inside the guest. Avocado
Virt is aimed at higher-level tests involving guests too:
https://avocado-virt.readthedocs.io/en/latest/WritingTests.html
Stefan
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v6 00/10] qemu.py: Pylint/style fixes
2017-08-24 12:15 ` Stefan Hajnoczi
@ 2017-08-24 12:49 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 25+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-08-24 12:49 UTC (permalink / raw)
To: Stefan Hajnoczi, Eduardo Habkost
Cc: Markus Armbruster, John Snow, Lukáš Doktor, qemu-devel,
famz, apahim, mreitz, Daniel P. Berrange, Kevin Wolf,
Paolo Bonzini, Cleber Rosa
On 08/24/2017 09:15 AM, Stefan Hajnoczi wrote:
> On Tue, Aug 22, 2017 at 04:07:09PM -0300, Eduardo Habkost wrote:
>> (CCing Cleber and Stefan)
>>
>> On Tue, Aug 22, 2017 at 07:19:45AM -0300, Philippe Mathieu-Daudé wrote:
>> [...]
>>> Can we predict how the python scripts will evolve? Only fast-testing?
>>>
>>
>> I guess it depends on how you define "fast". Does "fast-testing"
>> include a full device-crash-test run (that could take ~30
>> minutes) or the full set of iotests?
>>
>>
>>> Is there some users hacking on qemu.py unaware they can/should use
>>> libvirt-python?
>>
>> I believe the existing users of qemu.py wouldn't want to have
>> dependencies on libvirt or other external modules (e.g. code that
>> test specific QMP commands and/or is run by "make check").
>
> Yes, most QEMU tests interact with QEMU at a lower level than the
> libvirt API. They may exercise new features that are not available via
> libvirt so we'd end up bypassing it anyway.
good point,
> It is easier to debug a test case that interacts directly with QEMU.
> Investigating a failed test where libvirt is involved is more
> time-consuming.
another good point.
>
> libvirt and tools built on it, like virt-builder, are good for tests
> that need a fully-fledged operating system inside the guest. Avocado
> Virt is aimed at higher-level tests involving guests too:
> https://avocado-virt.readthedocs.io/en/latest/WritingTests.html
>
> Stefan
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v6 00/10] qemu.py: Pylint/style fixes
2017-08-22 7:24 ` Markus Armbruster
2017-08-22 10:19 ` Philippe Mathieu-Daudé
@ 2017-08-22 13:56 ` Paolo Bonzini
2017-08-22 18:11 ` John Snow
2017-08-22 18:51 ` Eduardo Habkost
3 siblings, 0 replies; 25+ messages in thread
From: Paolo Bonzini @ 2017-08-22 13:56 UTC (permalink / raw)
To: Markus Armbruster, John Snow
Cc: Lukáš Doktor, qemu-devel, famz, ehabkost, apahim, f4bug,
mreitz, Daniel P. Berrange, Kevin Wolf
On 22/08/2017 09:24, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:
>
>> On 08/18/2017 10:26 AM, Lukáš Doktor wrote:
>>> Hello guys,
>>>
>>> I'm reading the available python modules to exercise qemu and while reading them
>>> I fixed some issues that caught my attention. It usually starts with a simple
>>> pylint/docstring fixes and slowly graduates to more controversial ones so I'm
>>> open to suggestion to remove some of them.
>>>
>>> Kind regards,
>>> Lukáš
>>>
>>> Changes in v2
>>> - Squashed 2nd and 10th patches into 2nd one
>>> - Use repr() in MonitorResponseError's description
>>> - Improved commit message of the 6th patch
>>> - Two tweaks to docstrings changed in the 6th patch
>>> - Also updated qmp-shell to use new-style super calls (7th patch)
>>> - Fixed the accidental rename of qmp `cmd_id` (kept the id->cmd_id patch)
>>> - Changed the style of the style-fix in the 10th commit
>>>
>>> Changes in v3
>>> - Don't use repr in the 5th patch in MonitorResponseError
>>>
>>> Changes in v4
>>> - Use correct git base (remove unwanted commits)
>>>
>>> Changes in v5
>>> - Avoid bool comparison
>>> - Change report to return in one docstring
>>> - Removed the unnecessary spaces around single-line docstring
>>>
>>> Changes in v6
>>> - Bunch of docstring tweaks by Markus Armbruster
>>> - Line break in <80 chars
>>> - result dict => response dict
>>> - Removed the "event_match" rename
>>>
>>
>> Looks like all ten patches have an R-B despite changes; but it looks
>> like nothing particularly major was changed anyway.
>>
>> Does this fall under Markus's jurisdiction?
>>
>> (Well, except for qtest.py which seemingly has double-extra-no
>> maintainer...!)
>
> qemu.py is about starting and controlling QEMU, commonly for testing
> purposes. It's related to QMP only by virtue of using QMP for control
> (well, what else could it use?); if that makes me maintainer, I'll soon
> maintain basically all tests :)
>
> As far as I can tell, qemu.py's main user is still qemu-iotests, via
> qtest.py. Dan factored it out to make it available for
> tests/migration/guestperf/.
>
> Options for maintaining qemu.py and qtest.py:
>
> * Maintain them with qemu-iotest
>
> Currently mainained with the block layer core, by Kevin and Max.
>
> - Keep it that way
>
> - Appoint qemu-iotest maintainer(s).
>
> * Maintain them separately, say as "Python qtest support", appoint
> maintainer(s)
>
> Dan appears to be a hot contender:
>
> $ scripts/get_maintainer.pl -f --git-blame scripts/qtest.py
> Fam Zheng <famz@redhat.com> (authored lines:71/110=65%,commits:1/3=33%)
> "Daniel P. Berrange" <berrange@redhat.com> (authored lines:39/110=35%,commits:2/3=67%)
> Max Reitz <mreitz@redhat.com> (commits:2/3=67%)
> Amit Shah <amit.shah@redhat.com> (commits:1/3=33%)
> Stefan Hajnoczi <stefanha@redhat.com> (commits:1/3=33%)
> qemu-devel@nongnu.org (open list:All patches CC here)
> $ scripts/get_maintainer.pl -f --git-blame scripts/qemu.py
> "Daniel P. Berrange" <berrange@redhat.com> (authored lines:217/229=95%,commits:2/4=50%)
> Eduardo Habkost <ehabkost@redhat.com> (authored lines:12/229=5%,commits:4/4=100%)
> Markus Armbruster <armbru@redhat.com> (commits:1/4=25%)
> Max Reitz <mreitz@redhat.com> (commits:1/4=25%)
> Amit Shah <amit.shah@redhat.com> (commits:1/4=25%)
> qemu-devel@nongnu.org (open list:All patches CC here)
>
> Eduardo made the mistake^W^W^Wgraciously volunteered to maintain
> scripts/qmp/qmp-shell, which is also used for testing. Perhaps he'd
> be willing to maintain these guys as well.
>
> * Do nothing
>
> Hope "somebody" will take pity and merge patches. A common value of
> "somebody" would be Paolo.
Paolo is disappearing for two weeks starting Saturday, so you have
plenty of time to find another solution. :)
Paolo
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v6 00/10] qemu.py: Pylint/style fixes
2017-08-22 7:24 ` Markus Armbruster
2017-08-22 10:19 ` Philippe Mathieu-Daudé
2017-08-22 13:56 ` Paolo Bonzini
@ 2017-08-22 18:11 ` John Snow
2017-08-22 18:51 ` Eduardo Habkost
3 siblings, 0 replies; 25+ messages in thread
From: John Snow @ 2017-08-22 18:11 UTC (permalink / raw)
To: Markus Armbruster
Cc: Lukáš Doktor, qemu-devel, famz, ehabkost, apahim, f4bug,
mreitz, Daniel P. Berrange, Kevin Wolf, Paolo Bonzini
On 08/22/2017 03:24 AM, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:
>
>> On 08/18/2017 10:26 AM, Lukáš Doktor wrote:
>>> Hello guys,
>>>
>>> I'm reading the available python modules to exercise qemu and while reading them
>>> I fixed some issues that caught my attention. It usually starts with a simple
>>> pylint/docstring fixes and slowly graduates to more controversial ones so I'm
>>> open to suggestion to remove some of them.
>>>
>>> Kind regards,
>>> Lukáš
>>>
>>> Changes in v2
>>> - Squashed 2nd and 10th patches into 2nd one
>>> - Use repr() in MonitorResponseError's description
>>> - Improved commit message of the 6th patch
>>> - Two tweaks to docstrings changed in the 6th patch
>>> - Also updated qmp-shell to use new-style super calls (7th patch)
>>> - Fixed the accidental rename of qmp `cmd_id` (kept the id->cmd_id patch)
>>> - Changed the style of the style-fix in the 10th commit
>>>
>>> Changes in v3
>>> - Don't use repr in the 5th patch in MonitorResponseError
>>>
>>> Changes in v4
>>> - Use correct git base (remove unwanted commits)
>>>
>>> Changes in v5
>>> - Avoid bool comparison
>>> - Change report to return in one docstring
>>> - Removed the unnecessary spaces around single-line docstring
>>>
>>> Changes in v6
>>> - Bunch of docstring tweaks by Markus Armbruster
>>> - Line break in <80 chars
>>> - result dict => response dict
>>> - Removed the "event_match" rename
>>>
>>
>> Looks like all ten patches have an R-B despite changes; but it looks
>> like nothing particularly major was changed anyway.
>>
>> Does this fall under Markus's jurisdiction?
>>
>> (Well, except for qtest.py which seemingly has double-extra-no
>> maintainer...!)
>
> qemu.py is about starting and controlling QEMU, commonly for testing
> purposes. It's related to QMP only by virtue of using QMP for control
> (well, what else could it use?); if that makes me maintainer, I'll soon
> maintain basically all tests :)
>
> As far as I can tell, qemu.py's main user is still qemu-iotests, via
> qtest.py. Dan factored it out to make it available for
> tests/migration/guestperf/.
>
> Options for maintaining qemu.py and qtest.py:
>
> * Maintain them with qemu-iotest
>
> Currently mainained with the block layer core, by Kevin and Max.
>
> - Keep it that way
>
> - Appoint qemu-iotest maintainer(s).
>
> * Maintain them separately, say as "Python qtest support", appoint
> maintainer(s)
>
> Dan appears to be a hot contender:
>
> $ scripts/get_maintainer.pl -f --git-blame scripts/qtest.py
> Fam Zheng <famz@redhat.com> (authored lines:71/110=65%,commits:1/3=33%)
> "Daniel P. Berrange" <berrange@redhat.com> (authored lines:39/110=35%,commits:2/3=67%)
> Max Reitz <mreitz@redhat.com> (commits:2/3=67%)
> Amit Shah <amit.shah@redhat.com> (commits:1/3=33%)
> Stefan Hajnoczi <stefanha@redhat.com> (commits:1/3=33%)
> qemu-devel@nongnu.org (open list:All patches CC here)
> $ scripts/get_maintainer.pl -f --git-blame scripts/qemu.py
> "Daniel P. Berrange" <berrange@redhat.com> (authored lines:217/229=95%,commits:2/4=50%)
> Eduardo Habkost <ehabkost@redhat.com> (authored lines:12/229=5%,commits:4/4=100%)
> Markus Armbruster <armbru@redhat.com> (commits:1/4=25%)
> Max Reitz <mreitz@redhat.com> (commits:1/4=25%)
> Amit Shah <amit.shah@redhat.com> (commits:1/4=25%)
> qemu-devel@nongnu.org (open list:All patches CC here)
>
> Eduardo made the mistake^W^W^Wgraciously volunteered to maintain
> scripts/qmp/qmp-shell, which is also used for testing. Perhaps he'd
> be willing to maintain these guys as well.
>
> * Do nothing
>
> Hope "somebody" will take pity and merge patches. A common value of
> "somebody" would be Paolo.
>
Should we have an appointed python czar? There's been a lot of talk over
the right way to migrate from python2 to 3 and other design questions
about our python scripts and utilities going forward.
Might be nice to have a dedicated python-person to have a bit of an eye
over all of it...
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v6 00/10] qemu.py: Pylint/style fixes
2017-08-22 7:24 ` Markus Armbruster
` (2 preceding siblings ...)
2017-08-22 18:11 ` John Snow
@ 2017-08-22 18:51 ` Eduardo Habkost
3 siblings, 0 replies; 25+ messages in thread
From: Eduardo Habkost @ 2017-08-22 18:51 UTC (permalink / raw)
To: Markus Armbruster
Cc: John Snow, Lukáš Doktor, qemu-devel, famz, apahim,
f4bug, mreitz, Daniel P. Berrange, Kevin Wolf, Paolo Bonzini
On Tue, Aug 22, 2017 at 09:24:16AM +0200, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:
>
> > On 08/18/2017 10:26 AM, Lukáš Doktor wrote:
> >> Hello guys,
> >>
> >> I'm reading the available python modules to exercise qemu and while reading them
> >> I fixed some issues that caught my attention. It usually starts with a simple
> >> pylint/docstring fixes and slowly graduates to more controversial ones so I'm
> >> open to suggestion to remove some of them.
> >>
> >> Kind regards,
> >> Lukáš
> >>
> >> Changes in v2
> >> - Squashed 2nd and 10th patches into 2nd one
> >> - Use repr() in MonitorResponseError's description
> >> - Improved commit message of the 6th patch
> >> - Two tweaks to docstrings changed in the 6th patch
> >> - Also updated qmp-shell to use new-style super calls (7th patch)
> >> - Fixed the accidental rename of qmp `cmd_id` (kept the id->cmd_id patch)
> >> - Changed the style of the style-fix in the 10th commit
> >>
> >> Changes in v3
> >> - Don't use repr in the 5th patch in MonitorResponseError
> >>
> >> Changes in v4
> >> - Use correct git base (remove unwanted commits)
> >>
> >> Changes in v5
> >> - Avoid bool comparison
> >> - Change report to return in one docstring
> >> - Removed the unnecessary spaces around single-line docstring
> >>
> >> Changes in v6
> >> - Bunch of docstring tweaks by Markus Armbruster
> >> - Line break in <80 chars
> >> - result dict => response dict
> >> - Removed the "event_match" rename
> >>
> >
> > Looks like all ten patches have an R-B despite changes; but it looks
> > like nothing particularly major was changed anyway.
> >
> > Does this fall under Markus's jurisdiction?
> >
> > (Well, except for qtest.py which seemingly has double-extra-no
> > maintainer...!)
>
> qemu.py is about starting and controlling QEMU, commonly for testing
> purposes. It's related to QMP only by virtue of using QMP for control
> (well, what else could it use?); if that makes me maintainer, I'll soon
> maintain basically all tests :)
>
> As far as I can tell, qemu.py's main user is still qemu-iotests, via
> qtest.py. Dan factored it out to make it available for
> tests/migration/guestperf/.
>
> Options for maintaining qemu.py and qtest.py:
>
> * Maintain them with qemu-iotest
>
> Currently mainained with the block layer core, by Kevin and Max.
>
> - Keep it that way
>
> - Appoint qemu-iotest maintainer(s).
>
> * Maintain them separately, say as "Python qtest support", appoint
> maintainer(s)
>
> Dan appears to be a hot contender:
>
> $ scripts/get_maintainer.pl -f --git-blame scripts/qtest.py
> Fam Zheng <famz@redhat.com> (authored lines:71/110=65%,commits:1/3=33%)
> "Daniel P. Berrange" <berrange@redhat.com> (authored lines:39/110=35%,commits:2/3=67%)
> Max Reitz <mreitz@redhat.com> (commits:2/3=67%)
> Amit Shah <amit.shah@redhat.com> (commits:1/3=33%)
> Stefan Hajnoczi <stefanha@redhat.com> (commits:1/3=33%)
> qemu-devel@nongnu.org (open list:All patches CC here)
> $ scripts/get_maintainer.pl -f --git-blame scripts/qemu.py
> "Daniel P. Berrange" <berrange@redhat.com> (authored lines:217/229=95%,commits:2/4=50%)
> Eduardo Habkost <ehabkost@redhat.com> (authored lines:12/229=5%,commits:4/4=100%)
> Markus Armbruster <armbru@redhat.com> (commits:1/4=25%)
> Max Reitz <mreitz@redhat.com> (commits:1/4=25%)
> Amit Shah <amit.shah@redhat.com> (commits:1/4=25%)
> qemu-devel@nongnu.org (open list:All patches CC here)
>
> Eduardo made the mistake^W^W^Wgraciously volunteered to maintain
> scripts/qmp/qmp-shell, which is also used for testing. Perhaps he'd
> be willing to maintain these guys as well.
I could take care of queueing patches for the Python modules on
the same tree where qmp-shell patches would be queued. However,
I would be happier if somebody else agreed to be listed as
co-maintainer to at least help on review and decision-making.
--
Eduardo
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v6 00/10] qemu.py: Pylint/style fixes
2017-08-18 14:26 [Qemu-devel] [PATCH v6 00/10] qemu.py: Pylint/style fixes Lukáš Doktor
` (10 preceding siblings ...)
2017-08-21 21:51 ` [Qemu-devel] [PATCH v6 00/10] qemu.py: Pylint/style fixes John Snow
@ 2017-08-30 21:18 ` Eduardo Habkost
11 siblings, 0 replies; 25+ messages in thread
From: Eduardo Habkost @ 2017-08-30 21:18 UTC (permalink / raw)
To: Lukáš Doktor
Cc: qemu-devel, famz, apahim, armbru, mreitz, jsnow, f4bug
On Fri, Aug 18, 2017 at 04:26:03PM +0200, Lukáš Doktor wrote:
> Hello guys,
>
> I'm reading the available python modules to exercise qemu and while reading them
> I fixed some issues that caught my attention. It usually starts with a simple
> pylint/docstring fixes and slowly graduates to more controversial ones so I'm
> open to suggestion to remove some of them.
>
> Kind regards,
> Lukáš
I'm queueing this series on a python-next branch at:
https://github.com/ehabkost/qemu.git python-next
--
Eduardo
^ permalink raw reply [flat|nested] 25+ messages in thread