A few new annoyances. Of note is the new warning for an unspecified
encoding when opening a text file, which actually does indicate a
potentially real problem; see
https://www.python.org/dev/peps/pep-0597/#motivation

Use LC_CTYPE to determine an encoding to use for interpreting QEMU's
terminal output. Note that Python states: "language code and encoding
may be None if their values cannot be determined" -- use a platform
default as a backup.

Notes: Passing encoding=None will generate a suppressed warning on
Python 3.10+ that 'None' should not be passed as the encoding
argument. This behavior may be deprecated in the future and the default
switched to be a ubiquitous UTF-8. Opting in to the locale default will
be done by passing the encoding 'locale', but that isn't available in
3.6 through 3.9. Presumably this warning will be unsuppressed some time
prior to the actual switch and we can re-investigate these issues at
that time if necessary.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/qemu/machine/machine.py | 7 ++++++-
 python/setup.cfg               | 1 +
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
index a7081b1845..34131884a5 100644
--- a/python/qemu/machine/machine.py
+++ b/python/qemu/machine/machine.py
@@ -19,6 +19,7 @@
 
 import errno
 from itertools import chain
+import locale
 import logging
 import os
 import shutil
@@ -290,8 +291,12 @@ def get_pid(self) -> Optional[int]:
         return self._subp.pid
 
     def _load_io_log(self) -> None:
+        # Assume that the output encoding of QEMU's terminal output is
+        # defined by our locale. If indeterminate, allow open() to fall
+        # back to the platform default.
+        _, encoding = locale.getlocale()
         if self._qemu_log_path is not None:
-            with open(self._qemu_log_path, "r") as iolog:
+            with open(self._qemu_log_path, "r", encoding=encoding) as iolog:
                 self._iolog = iolog.read()
 
     @property
diff --git a/python/setup.cfg b/python/setup.cfg
index 83909c1c97..0f0cab098f 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -104,6 +104,7 @@ good-names=i,
 [pylint.similarities]
 # Ignore imports when computing similarities.
 ignore-imports=yes
+ignore-signatures=yes
 
 # Minimum lines number of a similarity.
 # TODO: Remove after we opt in to Pylint 2.8.3. See commit msg.
-- 
2.31.1

On Thu, Sep 16, 2021 at 02:22:47PM -0400, John Snow wrote:
> A few new annoyances. Of note is the new warning for an unspecified
> encoding when opening a text file, which actually does indicate a
> potentially real problem; see
> https://www.python.org/dev/peps/pep-0597/#motivation
> 
> Use LC_CTYPE to determine an encoding to use for interpreting QEMU's
> terminal output. Note that Python states: "language code and encoding
> may be None if their values cannot be determined" -- use a platform
> default as a backup.
> 
> Notes: Passing encoding=None will generate a suppressed warning on
> Python 3.10+ that 'None' should not be passed as the encoding
> argument. This behavior may be deprecated in the future and the default
> switched to be a ubiquitous UTF-8. Opting in to the locale default will
> be done by passing the encoding 'locale', but that isn't available in
> 3.6 through 3.9. Presumably this warning will be unsuppressed some time
> prior to the actual switch and we can re-investigate these issues at
> that time if necessary.

So, in the very worst case this will trigger a warning that is
currently suppressed.  And that will happen only if we are in the
unlikely situation where we have absolutely no information about
the encoding being used by other parts of the system.

Sounds reasonable to me, so:

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  python/qemu/machine/machine.py | 7 ++++++-
>  python/setup.cfg               | 1 +
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
> index a7081b1845..34131884a5 100644
> --- a/python/qemu/machine/machine.py
> +++ b/python/qemu/machine/machine.py
> @@ -19,6 +19,7 @@
>  
>  import errno
>  from itertools import chain
> +import locale
>  import logging
>  import os
>  import shutil
> @@ -290,8 +291,12 @@ def get_pid(self) -> Optional[int]:
>          return self._subp.pid
>  
>      def _load_io_log(self) -> None:
> +        # Assume that the output encoding of QEMU's terminal output is
> +        # defined by our locale. If indeterminate, allow open() to fall
> +        # back to the platform default.
> +        _, encoding = locale.getlocale()
>          if self._qemu_log_path is not None:
> -            with open(self._qemu_log_path, "r") as iolog:
> +            with open(self._qemu_log_path, "r", encoding=encoding) as iolog:
>                  self._iolog = iolog.read()
>  
>      @property
> diff --git a/python/setup.cfg b/python/setup.cfg
> index 83909c1c97..0f0cab098f 100644
> --- a/python/setup.cfg
> +++ b/python/setup.cfg
> @@ -104,6 +104,7 @@ good-names=i,
>  [pylint.similarities]
>  # Ignore imports when computing similarities.
>  ignore-imports=yes
> +ignore-signatures=yes
>  
>  # Minimum lines number of a similarity.
>  # TODO: Remove after we opt in to Pylint 2.8.3. See commit msg.
> -- 
> 2.31.1
> 

-- 
Eduardo

On Thu, Sep 16, 2021 at 3:29 PM John Snow <jsnow@redhat.com> wrote:
>
> A few new annoyances. Of note is the new warning for an unspecified
> encoding when opening a text file, which actually does indicate a
> potentially real problem; see
> https://www.python.org/dev/peps/pep-0597/#motivation
>
> Use LC_CTYPE to determine an encoding to use for interpreting QEMU's
> terminal output. Note that Python states: "language code and encoding
> may be None if their values cannot be determined" -- use a platform
> default as a backup.
>
> Notes: Passing encoding=None will generate a suppressed warning on
> Python 3.10+ that 'None' should not be passed as the encoding
> argument. This behavior may be deprecated in the future and the default
> switched to be a ubiquitous UTF-8. Opting in to the locale default will
> be done by passing the encoding 'locale', but that isn't available in
> 3.6 through 3.9. Presumably this warning will be unsuppressed some time
> prior to the actual switch and we can re-investigate these issues at
> that time if necessary.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  python/qemu/machine/machine.py | 7 ++++++-
>  python/setup.cfg               | 1 +
>  2 files changed, 7 insertions(+), 1 deletion(-)
>

Reviewed-by: Willian Rampazzo <willianr@redhat.com>