Hi,
On 6/8/21 11:09 AM, Cleber Rosa wrote:
> Which can be used to check for any "feature" that is
available as a
> QEMU command line option, and that will return its list
of available
> options.
>
> This is a generalization of the list_accel() utility
function, which
> is itself re-implemented in terms of the more generic
feature.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
> python/qemu/utils/__init__.py | 2 ++
> python/qemu/utils/accel.py | 15 ++----------
> python/qemu/utils/feature.py | 44
+++++++++++++++++++++++++++++++++++
> 3 files changed, 48 insertions(+), 13 deletions(-)
> create mode 100644 python/qemu/utils/feature.py
>
> diff --git a/python/qemu/utils/__init__.py
b/python/qemu/utils/__init__.py
> index 7f1a5138c4..1d0789eaa2 100644
> --- a/python/qemu/utils/__init__.py
> +++ b/python/qemu/utils/__init__.py
> @@ -20,12 +20,14 @@
>
> # pylint: disable=import-error
> from .accel import kvm_available, list_accel,
tcg_available
> +from .feature import list_feature
>
>
> __all__ = (
> 'get_info_usernet_hostfwd_port',
> 'kvm_available',
> 'list_accel',
> + 'list_feature',
> 'tcg_available',
> )
>
> diff --git a/python/qemu/utils/accel.py
b/python/qemu/utils/accel.py
> index 297933df2a..b5bb80c6d3 100644
> --- a/python/qemu/utils/accel.py
> +++ b/python/qemu/utils/accel.py
> @@ -14,13 +14,11 @@
> # the COPYING file in the top-level directory.
> #
>
> -import logging
> import os
> -import subprocess
> from typing import List, Optional
>
> +from qemu.utils.feature import list_feature
>
> -LOG = logging.getLogger(__name__)
>
> # Mapping host architecture to any additional
architectures it can
> # support which often includes its 32 bit cousin.
> @@ -39,16 +37,7 @@ def list_accel(qemu_bin: str) ->
List[str]:
> @raise Exception: if failed to run `qemu -accel
help`
> @return a list of accelerator names.
> """
> - if not qemu_bin:
> - return []
> - try:
> - out = subprocess.check_output([qemu_bin,
'-accel', 'help'],
> -
universal_newlines=True)
> - except:
> - LOG.debug("Failed to get the list of
accelerators in %s", qemu_bin)
> - raise
> - # Skip the first line which is the header.
> - return [acc.strip() for acc in
out.splitlines()[1:]]
> + return list_feature(qemu_bin, 'accel')
>
>
> def kvm_available(target_arch: Optional[str] = None,
> diff --git a/python/qemu/utils/feature.py
b/python/qemu/utils/feature.py
> new file mode 100644
> index 0000000000..b4a5f929ab
> --- /dev/null
> +++ b/python/qemu/utils/feature.py
> @@ -0,0 +1,44 @@
> +"""
> +QEMU feature module:
> +
> +This module provides a utility for discovering the
availability of
> +generic features.
> +"""
> +# Copyright (C) 2022 Red Hat Inc.
Cleber, please, tell me how is the future like! :)
I grabbed a sports almanac. That's all I can say. :)
Now seriously, thanks for spotting the typo.
> +#
> +# Authors:
> +# Cleber Rosa <crosa@redhat.com>
> +#
> +# This work is licensed under the terms of the GNU
GPL, version 2. See
> +# the COPYING file in the top-level directory.
> +#
> +
> +import logging
> +import subprocess
> +from typing import List
> +
> +
> +LOG = logging.getLogger(__name__)
> +
> +
> +def list_feature(qemu_bin: str, feature: str) ->
List[str]:
> + """
> + List available options the QEMU binary for a given
feature type.
> +
> + By calling a "qemu $feature -help" and parsing its
output.
I understand we need a mean to easily cancel the test if
given feature
is not present. However, I'm unsure this generic
list_feature() is what
we need.
The `-accel help` returns a simple list of strings (besides
the header,
which is dismissed). Whereas `-machine help` returns what
could be
parsed as a tuple of (name, description).
Another example is `-cpu help` which will print a similar
list as
`-machine`, plus a section with CPUID flags.
I made sure it worked with both "accel" and "machine",
but you're right about many other "-$feature help" that
won't conform to the mapping ("-chardev help" is probably
the only other one that should work). What I thought about
was to keep the same list_feature(), but make its parsing of
items flexible. Then it could be reused for other
list_$feature() like methods. At the same time, it could be
an opportunity to standardize a bit more of the "help"
outputs.
For instance, I think it would make sense for "cpu" to
keep showing the amount of information it shows, but:
1) The first item could be the name of the relevant
"option" (the cpu model) for that feature (cpu), instead of,
say, "x86". Basically reversing the order of first and
second, or first and third fields.
2) Everything *after* an empty line would be extra
information, not necessarily an option available for that
feature.
But, this is most probably not going to be achieved in
the short term.
What I can do here, is to hide list_feature(), say call
it _list_feature() so that we don't duplicate code, and
expose a list_machine().