Re: [PATCH 2/4] Python QEMU utils: introduce a generic feature list

[Wainer dos Santos Moschetta <wainersm@redhat.com>]

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! :)
> +#
> +# 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.

If confess I still don't have a better idea, although I feel it will 
require a OO design.

Thanks!

- Wainer

> +
> +    @param qemu_bin (str): path to the QEMU binary.
> +    @param feature (str): feature name, matching the command line option.
> +    @raise Exception: if failed to run `qemu -feature help`
> +    @return a list of available options for the given feature.
> +    """
> +    if not qemu_bin:
> +        return []
> +    try:
> +        out = subprocess.check_output([qemu_bin, '-%s' % feature, 'help'],
> +                                      universal_newlines=True)
> +    except:
> +        LOG.debug("Failed to get the list of %s(s) in %s", feature, qemu_bin)
> +        raise
> +    # Skip the first line which is the header.
> +    return [item.split(' ', 1)[0] for item in out.splitlines()[1:]]