| Message ID | 20210608140938.863580-3-crosa@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Jobs based on custom runners: add CentOS Stream 8 | expand |
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:]]
On Tue, Jun 8, 2021 at 5:42 PM Wainer dos Santos Moschetta < wainersm@redhat.com> 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 <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(). 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:]] > >
On Tue, Jun 8, 2021 at 8:55 PM Cleber Rosa Junior <crosa@redhat.com> wrote: > > > > On Tue, Jun 8, 2021 at 5:42 PM Wainer dos Santos Moschetta <wainersm@redhat.com> 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 <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(). I have reviewed the code and it looks good to me, but +1 for `list_machine()` and other `list_<specific>` functions. We can handle different cases on its own function and make it easier to use. > > 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:]] >>
On Tue, Jun 8, 2021 at 11:09 AM Cleber Rosa <crosa@redhat.com> 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 > Based on my comments from the next patch of this series: Reviewed-by: Willian Rampazzo <willianr@redhat.com>
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 > <wainersm@redhat.com <mailto:wainersm@redhat.com>> 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 <crosa@redhat.com > <mailto: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 <mailto: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(). 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:]] >
On 6/8/21 10: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. > +# > +# 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. > + > + @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:]] > It's messy stuff, but all of machine.py is pretty messy stuff right now. No real qualms with more messy stuff going into qemu.utils for the time being. Eventually, we will want to come up with a more universal way to interrogate features present in QEMU binaries. Using introspection data or QMP queries would be my preferred (and ideally SOLE) way to detect QEMU features. But that's something to worry about later, I suppose. As long as it passes the CI and doesn't break any tests, I'll toss you my ACK here and trust your judgment: Acked-by: John Snow <jsnow@redhat.com> --js
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. +# +# 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. + + @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:]]
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