Hi, On 6/8/21 8:55 PM, Cleber Rosa Junior wrote: > > > On Tue, Jun 8, 2021 at 5:42 PM Wainer dos Santos Moschetta > > wrote: > > 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 > > > --- > >   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 > > > +# > > +# 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(). I prefer that implementation, with list_machine(), list_accel()...etc. Allow me another suggestion: maybe rename list_feature() to feature_help() (or something similar). - Wainer > > Let me know what you think. > > Thanks, > - Cleber. > > 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:]] >