* [PATCH v2 1/6] python/console_socket: avoid one-letter variable
2021-05-17 18:48 [PATCH v2 0/6] Python: delint python library John Snow
@ 2021-05-17 18:48 ` John Snow
2021-05-17 18:48 ` [PATCH v2 2/6] python/machine: use subprocess.DEVNULL instead of open(os.path.devnull) John Snow
` (5 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: John Snow @ 2021-05-17 18:48 UTC (permalink / raw)
To: qemu-devel
Cc: Emanuele Giuseppe Esposito, Eduardo Habkost, John Snow,
Wainer dos Santos Moschetta, Cleber Rosa,
Philippe Mathieu-Daudé
Fixes pylint warnings.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
python/qemu/console_socket.py | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/python/qemu/console_socket.py b/python/qemu/console_socket.py
index ac21130e446..87237bebef7 100644
--- a/python/qemu/console_socket.py
+++ b/python/qemu/console_socket.py
@@ -46,11 +46,11 @@ def __init__(self, address: str, file: Optional[str] = None,
self._drain_thread = self._thread_start()
def __repr__(self) -> str:
- s = super().__repr__()
- s = s.rstrip(">")
- s = "%s, logfile=%s, drain_thread=%s>" % (s, self._logfile,
- self._drain_thread)
- return s
+ tmp = super().__repr__()
+ tmp = tmp.rstrip(">")
+ tmp = "%s, logfile=%s, drain_thread=%s>" % (tmp, self._logfile,
+ self._drain_thread)
+ return tmp
def _drain_fn(self) -> None:
"""Drains the socket and runs while the socket is open."""
--
2.30.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 2/6] python/machine: use subprocess.DEVNULL instead of open(os.path.devnull)
2021-05-17 18:48 [PATCH v2 0/6] Python: delint python library John Snow
2021-05-17 18:48 ` [PATCH v2 1/6] python/console_socket: avoid one-letter variable John Snow
@ 2021-05-17 18:48 ` John Snow
2021-05-18 3:33 ` Cleber Rosa
2021-05-17 18:48 ` [PATCH v2 3/6] python/machine: use subprocess.run instead of subprocess.Popen John Snow
` (4 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: John Snow @ 2021-05-17 18:48 UTC (permalink / raw)
To: qemu-devel
Cc: Emanuele Giuseppe Esposito, Eduardo Habkost, John Snow,
Wainer dos Santos Moschetta, Cleber Rosa,
Philippe Mathieu-Daudé
One less file resource to manage, and it helps quiet some pylint >=
2.8.0 warnings about not using a with-context manager for the open call.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
python/qemu/machine.py | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index 6e44bda337e..41f51bd27d0 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -223,9 +223,8 @@ def send_fd_scm(self, fd: Optional[int] = None,
assert fd is not None
fd_param.append(str(fd))
- devnull = open(os.path.devnull, 'rb')
proc = subprocess.Popen(
- fd_param, stdin=devnull, stdout=subprocess.PIPE,
+ fd_param, stdin=subprocess.DEVNULL, stdout=subprocess.PIPE,
stderr=subprocess.STDOUT, close_fds=False
)
output = proc.communicate()[0]
@@ -393,7 +392,6 @@ def _launch(self) -> None:
"""
Launch the VM and establish a QMP connection
"""
- devnull = open(os.path.devnull, 'rb')
self._pre_launch()
self._qemu_full_args = tuple(
chain(self._wrapper,
@@ -403,7 +401,7 @@ def _launch(self) -> None:
)
LOG.debug('VM launch command: %r', ' '.join(self._qemu_full_args))
self._popen = subprocess.Popen(self._qemu_full_args,
- stdin=devnull,
+ stdin=subprocess.DEVNULL,
stdout=self._qemu_log_file,
stderr=subprocess.STDOUT,
shell=False,
--
2.30.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/6] python/machine: use subprocess.DEVNULL instead of open(os.path.devnull)
2021-05-17 18:48 ` [PATCH v2 2/6] python/machine: use subprocess.DEVNULL instead of open(os.path.devnull) John Snow
@ 2021-05-18 3:33 ` Cleber Rosa
0 siblings, 0 replies; 13+ messages in thread
From: Cleber Rosa @ 2021-05-18 3:33 UTC (permalink / raw)
To: John Snow
Cc: Emanuele Giuseppe Esposito, Philippe Mathieu-Daudé,
qemu-devel, Wainer dos Santos Moschetta, Eduardo Habkost
[-- Attachment #1: Type: text/plain, Size: 464 bytes --]
On Mon, May 17, 2021 at 02:48:04PM -0400, John Snow wrote:
> One less file resource to manage, and it helps quiet some pylint >=
> 2.8.0 warnings about not using a with-context manager for the open call.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> python/qemu/machine.py | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 3/6] python/machine: use subprocess.run instead of subprocess.Popen
2021-05-17 18:48 [PATCH v2 0/6] Python: delint python library John Snow
2021-05-17 18:48 ` [PATCH v2 1/6] python/console_socket: avoid one-letter variable John Snow
2021-05-17 18:48 ` [PATCH v2 2/6] python/machine: use subprocess.DEVNULL instead of open(os.path.devnull) John Snow
@ 2021-05-17 18:48 ` John Snow
2021-05-18 3:48 ` Cleber Rosa
2021-05-17 18:48 ` [PATCH v2 4/6] python/console_socket: Add a pylint ignore John Snow
` (3 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: John Snow @ 2021-05-17 18:48 UTC (permalink / raw)
To: qemu-devel
Cc: Emanuele Giuseppe Esposito, John Snow, Eduardo Habkost,
Wainer dos Santos Moschetta, Cleber Rosa
use run() instead of Popen() -- to assert to pylint that we are not
forgetting to close a long-running program.
Signed-off-by: John Snow <jsnow@redhat.com>
---
python/qemu/machine.py | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index 41f51bd27d0..2e55e2c8bd8 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -223,13 +223,16 @@ def send_fd_scm(self, fd: Optional[int] = None,
assert fd is not None
fd_param.append(str(fd))
- proc = subprocess.Popen(
- fd_param, stdin=subprocess.DEVNULL, stdout=subprocess.PIPE,
- stderr=subprocess.STDOUT, close_fds=False
+ proc = subprocess.run(
+ fd_param,
+ stdin=subprocess.DEVNULL,
+ stdout=subprocess.PIPE,
+ stderr=subprocess.STDOUT,
+ check=False,
+ close_fds=False,
)
- output = proc.communicate()[0]
- if output:
- LOG.debug(output)
+ if proc.stdout:
+ LOG.debug(proc.stdout)
return proc.returncode
--
2.30.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/6] python/machine: use subprocess.run instead of subprocess.Popen
2021-05-17 18:48 ` [PATCH v2 3/6] python/machine: use subprocess.run instead of subprocess.Popen John Snow
@ 2021-05-18 3:48 ` Cleber Rosa
0 siblings, 0 replies; 13+ messages in thread
From: Cleber Rosa @ 2021-05-18 3:48 UTC (permalink / raw)
To: John Snow
Cc: Emanuele Giuseppe Esposito, qemu-devel,
Wainer dos Santos Moschetta, Eduardo Habkost
[-- Attachment #1: Type: text/plain, Size: 474 bytes --]
On Mon, May 17, 2021 at 02:48:05PM -0400, John Snow wrote:
> use run() instead of Popen() -- to assert to pylint that we are not
> forgetting to close a long-running program.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> python/qemu/machine.py | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
qemu-iotests 045 and 147 are happy, so:
Tested-by: Cleber Rosa <crosa@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 4/6] python/console_socket: Add a pylint ignore
2021-05-17 18:48 [PATCH v2 0/6] Python: delint python library John Snow
` (2 preceding siblings ...)
2021-05-17 18:48 ` [PATCH v2 3/6] python/machine: use subprocess.run instead of subprocess.Popen John Snow
@ 2021-05-17 18:48 ` John Snow
2021-05-18 3:50 ` Cleber Rosa
2021-05-17 18:48 ` [PATCH v2 5/6] python/machine: Disable pylint warning for open() in _pre_launch John Snow
` (2 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: John Snow @ 2021-05-17 18:48 UTC (permalink / raw)
To: qemu-devel
Cc: Emanuele Giuseppe Esposito, John Snow, Eduardo Habkost,
Wainer dos Santos Moschetta, Cleber Rosa
We manage cleaning up this resource ourselves. Pylint should shush.
Signed-off-by: John Snow <jsnow@redhat.com>
---
python/qemu/console_socket.py | 1 +
1 file changed, 1 insertion(+)
diff --git a/python/qemu/console_socket.py b/python/qemu/console_socket.py
index 87237bebef7..8c4ff598ad7 100644
--- a/python/qemu/console_socket.py
+++ b/python/qemu/console_socket.py
@@ -39,6 +39,7 @@ def __init__(self, address: str, file: Optional[str] = None,
self.connect(address)
self._logfile = None
if file:
+ # pylint: disable=consider-using-with
self._logfile = open(file, "bw")
self._open = True
self._drain_thread = None
--
2.30.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 4/6] python/console_socket: Add a pylint ignore
2021-05-17 18:48 ` [PATCH v2 4/6] python/console_socket: Add a pylint ignore John Snow
@ 2021-05-18 3:50 ` Cleber Rosa
0 siblings, 0 replies; 13+ messages in thread
From: Cleber Rosa @ 2021-05-18 3:50 UTC (permalink / raw)
To: John Snow
Cc: Emanuele Giuseppe Esposito, qemu-devel,
Wainer dos Santos Moschetta, Eduardo Habkost
[-- Attachment #1: Type: text/plain, Size: 314 bytes --]
On Mon, May 17, 2021 at 02:48:06PM -0400, John Snow wrote:
> We manage cleaning up this resource ourselves. Pylint should shush.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> python/qemu/console_socket.py | 1 +
> 1 file changed, 1 insertion(+)
>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 5/6] python/machine: Disable pylint warning for open() in _pre_launch
2021-05-17 18:48 [PATCH v2 0/6] Python: delint python library John Snow
` (3 preceding siblings ...)
2021-05-17 18:48 ` [PATCH v2 4/6] python/console_socket: Add a pylint ignore John Snow
@ 2021-05-17 18:48 ` John Snow
2021-05-18 4:07 ` Cleber Rosa
2021-05-17 18:48 ` [PATCH v2 6/6] python/machine: disable warning for Popen in _launch() John Snow
2021-05-18 13:33 ` [PATCH v2 0/6] Python: delint python library John Snow
6 siblings, 1 reply; 13+ messages in thread
From: John Snow @ 2021-05-17 18:48 UTC (permalink / raw)
To: qemu-devel
Cc: Emanuele Giuseppe Esposito, John Snow, Eduardo Habkost,
Wainer dos Santos Moschetta, Cleber Rosa
Shift the open() call later so that the pylint pragma applies *only* to
that one open() call. Add a note that suggests why this is safe: the
resource is unconditionally cleaned up in _post_shutdown().
_post_shutdown is called after failed launches (see launch()), and
unconditionally after every call to shutdown(), and therefore also on
__exit__.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
---
python/qemu/machine.py | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index 2e55e2c8bd8..f021127f0fc 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -308,7 +308,6 @@ def _pre_launch(self) -> None:
self._temp_dir = tempfile.mkdtemp(prefix="qemu-machine-",
dir=self._test_dir)
self._qemu_log_path = os.path.join(self._temp_dir, self._name + ".log")
- self._qemu_log_file = open(self._qemu_log_path, 'wb')
if self._console_set:
self._remove_files.append(self._console_address)
@@ -323,6 +322,11 @@ def _pre_launch(self) -> None:
nickname=self._name
)
+ # NOTE: Make sure any opened resources are *definitely* freed in
+ # _post_shutdown()!
+ # pylint: disable=consider-using-with
+ self._qemu_log_file = open(self._qemu_log_path, 'wb')
+
def _post_launch(self) -> None:
if self._qmp_connection:
self._qmp.accept()
--
2.30.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 5/6] python/machine: Disable pylint warning for open() in _pre_launch
2021-05-17 18:48 ` [PATCH v2 5/6] python/machine: Disable pylint warning for open() in _pre_launch John Snow
@ 2021-05-18 4:07 ` Cleber Rosa
0 siblings, 0 replies; 13+ messages in thread
From: Cleber Rosa @ 2021-05-18 4:07 UTC (permalink / raw)
To: John Snow
Cc: Emanuele Giuseppe Esposito, qemu-devel,
Wainer dos Santos Moschetta, Eduardo Habkost
[-- Attachment #1: Type: text/plain, Size: 693 bytes --]
On Mon, May 17, 2021 at 02:48:07PM -0400, John Snow wrote:
> Shift the open() call later so that the pylint pragma applies *only* to
> that one open() call. Add a note that suggests why this is safe: the
> resource is unconditionally cleaned up in _post_shutdown().
>
> _post_shutdown is called after failed launches (see launch()), and
> unconditionally after every call to shutdown(), and therefore also on
> __exit__.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
> ---
> python/qemu/machine.py | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 6/6] python/machine: disable warning for Popen in _launch()
2021-05-17 18:48 [PATCH v2 0/6] Python: delint python library John Snow
` (4 preceding siblings ...)
2021-05-17 18:48 ` [PATCH v2 5/6] python/machine: Disable pylint warning for open() in _pre_launch John Snow
@ 2021-05-17 18:48 ` John Snow
2021-05-18 4:08 ` Cleber Rosa
2021-05-18 13:33 ` [PATCH v2 0/6] Python: delint python library John Snow
6 siblings, 1 reply; 13+ messages in thread
From: John Snow @ 2021-05-17 18:48 UTC (permalink / raw)
To: qemu-devel
Cc: Emanuele Giuseppe Esposito, John Snow, Eduardo Habkost,
Wainer dos Santos Moschetta, Cleber Rosa
We handle this resource rather meticulously in
shutdown/kill/wait/__exit__ et al, through the laborious mechanisms in
_do_shutdown().
Quiet this pylint warning here.
Signed-off-by: John Snow <jsnow@redhat.com>
---
python/qemu/machine.py | 3 +++
1 file changed, 3 insertions(+)
diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index f021127f0fc..06058d89e83 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -407,6 +407,9 @@ def _launch(self) -> None:
self._args)
)
LOG.debug('VM launch command: %r', ' '.join(self._qemu_full_args))
+
+ # Cleaning up of this subprocess is guaranteed by _do_shutdown.
+ # pylint: disable=consider-using-with
self._popen = subprocess.Popen(self._qemu_full_args,
stdin=subprocess.DEVNULL,
stdout=self._qemu_log_file,
--
2.30.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 6/6] python/machine: disable warning for Popen in _launch()
2021-05-17 18:48 ` [PATCH v2 6/6] python/machine: disable warning for Popen in _launch() John Snow
@ 2021-05-18 4:08 ` Cleber Rosa
0 siblings, 0 replies; 13+ messages in thread
From: Cleber Rosa @ 2021-05-18 4:08 UTC (permalink / raw)
To: John Snow
Cc: Emanuele Giuseppe Esposito, qemu-devel,
Wainer dos Santos Moschetta, Eduardo Habkost
[-- Attachment #1: Type: text/plain, Size: 421 bytes --]
On Mon, May 17, 2021 at 02:48:08PM -0400, John Snow wrote:
> We handle this resource rather meticulously in
> shutdown/kill/wait/__exit__ et al, through the laborious mechanisms in
> _do_shutdown().
>
> Quiet this pylint warning here.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> python/qemu/machine.py | 3 +++
> 1 file changed, 3 insertions(+)
>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/6] Python: delint python library
2021-05-17 18:48 [PATCH v2 0/6] Python: delint python library John Snow
` (5 preceding siblings ...)
2021-05-17 18:48 ` [PATCH v2 6/6] python/machine: disable warning for Popen in _launch() John Snow
@ 2021-05-18 13:33 ` John Snow
6 siblings, 0 replies; 13+ messages in thread
From: John Snow @ 2021-05-18 13:33 UTC (permalink / raw)
To: qemu-devel
Cc: Emanuele Giuseppe Esposito, Eduardo Habkost,
Wainer dos Santos Moschetta, Cleber Rosa
On 5/17/21 2:48 PM, John Snow wrote:
> gitlab CI: https://gitlab.com/jsnow/qemu/-/pipelines/304224309
> branch: https://gitlab.com/jsnow/qemu/-/commits/python-package-pre-cleanup
>
> This series serves as a pre-requisite for packaging the python series
> and getting the linters running via CI. The first patch fixes a linter
> error we've had for a while now; the subsequent ones fix a new warning
> that was recently added to pylint 2.8.x.
>
> V2:
> - iotest bits already handled, dropped here.
> - Changed patch #3 based on feedback from Wainer.
>
> John Snow (6):
> python/console_socket: avoid one-letter variable
> python/machine: use subprocess.DEVNULL instead of
> open(os.path.devnull)
> python/machine: use subprocess.run instead of subprocess.Popen
> python/console_socket: Add a pylint ignore
> python/machine: Disable pylint warning for open() in _pre_launch
> python/machine: disable warning for Popen in _launch()
>
> python/qemu/console_socket.py | 11 ++++++-----
> python/qemu/machine.py | 28 ++++++++++++++++++----------
> 2 files changed, 24 insertions(+), 15 deletions(-)
>
Thank you for reviews! Staged to my Python branch.
--js
^ permalink raw reply [flat|nested] 13+ messages in thread