Am 07.10.2020 um 01:58 hat John Snow geschrieben: > Put the init arg handling all at the top, and mostly in order (deviating > when one is dependent on another), and put what is effectively runtime > state declaration at the bottom. > > Signed-off-by: John Snow <js...@redhat.com> > --- > python/qemu/machine.py | 44 ++++++++++++++++++++++++------------------ > 1 file changed, 25 insertions(+), 19 deletions(-) > > diff --git a/python/qemu/machine.py b/python/qemu/machine.py > index 3017ec072df..71fe58be050 100644 > --- a/python/qemu/machine.py > +++ b/python/qemu/machine.py > @@ -84,42 +84,54 @@ def __init__(self, binary, args=None, wrapper=None, > name=None, > @param monitor_address: address for QMP monitor > @param socket_scm_helper: helper program, required for send_fd_scm() > @param sock_dir: where to create socket (overrides test_dir for sock) > - @param console_log: (optional) path to console log file > @param drain_console: (optional) True to drain console socket to > buffer > + @param console_log: (optional) path to console log file > @note: Qemu process is not started until launch() is used. > ''' > + # Direct user configuration > + > + self._binary = binary > + > if args is None: > args = [] > + # Copy mutable input: we will be modifying our copy > + self._args = list(args) > + > if wrapper is None: > wrapper = [] > - if name is None: > - name = "qemu-%d" % os.getpid() > - if sock_dir is None: > - sock_dir = test_dir > - self._name = name > + self._wrapper = wrapper > + > + self._name = name or "qemu-%d" % os.getpid() > + self._test_dir = test_dir > + self._sock_dir = sock_dir or self._test_dir > + self._socket_scm_helper = socket_scm_helper
Interesting that you use a shortcut with 'or' for name and sock_dir, but you don't have 'wraper or []' and 'args or []' above. It's not wrong, of course, but if you have to respin for another reason, maybe an inconsistency to address. Reviewed-by: Kevin Wolf <kw...@redhat.com>