On Wed, Dec 11, 2019 at 02:58:35PM -0200, Wainer dos Santos Moschetta wrote:
>
> On 12/9/19 10:52 PM, Cleber Rosa wrote:
> > On Fri, Dec 06, 2019 at 04:34:31PM -0500, Wainer dos Santos Moschetta wrote:
> > > Since commit cbe6d6365a48 the command `qemu -accel help` returns
> > > the list of accelerators enabled in the QEMU binary. This adds
> > > the list_accel() method which return that same list.
> > >
> > > Signed-off-by: Wainer dos Santos Moschetta <waine...@redhat.com>
> > > Reviewed-by: Alex Bennée <alex.ben...@linaro.org>
> > > Reviewed-by: Philippe Mathieu-Daudé <phi...@redhat.com>
> > > ---
> > > python/qemu/accel.py | 23 +++++++++++++++++++++++
> > > 1 file changed, 23 insertions(+)
> > >
> > > diff --git a/python/qemu/accel.py b/python/qemu/accel.py
> > > index cbeac10dd1..746b7e68f5 100644
> > > --- a/python/qemu/accel.py
> > > +++ b/python/qemu/accel.py
> > > @@ -14,7 +14,11 @@ accelerators.
> > > # the COPYING file in the top-level directory.
> > > #
> > > +import logging
> > > import os
> > > +import subprocess
> > > +
> > > +LOG = logging.getLogger(__name__)
> > > # Mapping host architecture to any additional architectures it can
> > > # support which often includes its 32 bit cousin.
> > > @@ -23,6 +27,25 @@ ADDITIONAL_ARCHES = {
> > > "aarch64" : "armhf"
> > > }
> > > +def list_accel(qemu_bin):
> > > + """
> > > + List accelerators enabled in the QEMU binary.
> > > +
> > > + @param qemu_bin (str): path to the QEMU binary.
> > > + @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("%s -accel help" % qemu_bin,
> > > shell=True)
> > There's no need to use a shell here. This could become:
> >
> > out = subprocess.check_output([qemu_bin, '-accel' 'help'])
>
> Ack
>
> >
> > > + except:
> > > + LOG.debug("Failed to get the list of accelerators in %s" %
> > > qemu_bin)
> > > + raise
> > > + lines = out.decode().splitlines()
> > And maybe discard the first line earlier with:
> >
> > lines = out.decode().splitlines()[1:]
> >
> > Also, you could avoid the manual decode() with the `universal_newlines`
> > option to subprocess.check_output(), ie:
> >
> > accels = subprocess.check_output([qemu-bin, '-accel', 'help'],
> >
> > universal_newlines=True).splitlines()[1:]
>
> Nice. v3 will have universal_newlines=True.
>
> >
> > > + # Skip the first line which is the header.
> > > + return [l.strip() for l in lines[1:] if l]
> > > +
> > I think that the `if l` check can actually hide undesirable behavior
> > (bugs) in the `qemu -accel ?` output. I don't remember seeing
> > `-$(option) ?` returning empty strings but doesn't mean it couldn't
> > and shouldn't).
> >
> > I do remember `-machine ?` returning random non-printable characters
> > that turned out to be a bug, though.
>
> Double-checking: are you suggesting to remove the 'if not empty' check so
> that bugs on output could emerge?
>Yes, that's my suggestion. I don't think we need to process QEMU's output beyond what it's expected to be returned. Cheers, - Cleber. > Thanks! > > - Wainer > > > > > > def kvm_available(target_arch=None): > > > host_arch = os.uname()[4] > > > if target_arch and target_arch != host_arch: > > > -- > > > 2.21.0 > > > > > - Cleber. > >
signature.asc
Description: PGP signature
