Re: [PATCH v2 03/29] python/qemu: Add binutils::binary_get_version()

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 2/3/20 8:17 PM, Wainer dos Santos Moschetta wrote:
> 
> On 1/29/20 7:23 PM, Philippe Mathieu-Daudé wrote:
>> Add a helper to query the version of a QEMU binary.
>> We simply send the 'query-version' command over a QMP
>> socket.
>> Introduce the PythonQemuCoreScripts class to test our
>> new helper.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>   python/qemu/binutils.py          | 38 ++++++++++++++++++++++++++++++++
> 
> I'm not sure about creating the file with that name, it reminds me to 
> GNU Binutils rather than the QEMU binary. :)
> 
> Perhaps it could be named qemu_bin.py?

OK, I don't mind.

> 
> Another suggestion is to encapsulate the methods you propose in this 
> series in an object. For example:
> 
> class QEMUBin:
>      def __init__(self, bin_path):
>          # Check bin exists.
>          self.bin_path = bin_path
> 
>      def get_version(self):
>          # binutils.binary_get_version() goes here.
>          pass
> 
>      def get_arch(self):
>          # binutils.binary_get_arch() goes here.
>          pass
> 
>      def list_accel(self):
>          # move accel.list_accel() method to here.
>          pass
> 
>      def get_vm(self, args):
>          # Return an QEMUMachine object...
>          return QEMUMachine(self.bin_path, *args)
> 
>      def get_build_config_host(self):
>          # Detect if self.bin_path is in a build directory,
>          # attempt to read the host-config.mak and return
>          # as hash. Or fail...
>          pass

OK, I'll try.

> 
>>   tests/acceptance/core_scripts.py | 31 ++++++++++++++++++++++++++
>>   2 files changed, 69 insertions(+)
>>   create mode 100644 python/qemu/binutils.py
>>   create mode 100644 tests/acceptance/core_scripts.py
>>
>> diff --git a/python/qemu/binutils.py b/python/qemu/binutils.py
>> new file mode 100644
>> index 0000000000..96b200eef4
>> --- /dev/null
>> +++ b/python/qemu/binutils.py
>> @@ -0,0 +1,38 @@
>> +"""
>> +QEMU binary utility module:
>> +
>> +The binary utility module provides helpers to query QEMU binary for
>> +build-dependent configuration options at runtime.
>> +"""
>> +#
>> +# Copyright (c) 2020 Red Hat, Inc.
>> +#
>> +# Author:
>> +#  Philippe Mathieu-Daudé <philmd@redhat.com>
>> +#
>> +# This work is licensed under the terms of the GNU GPL, version 2 or 
>> later.
>> +# See the COPYING file in the top-level directory.
>> +#
>> +# SPDX-License-Identifier: GPL-2.0-or-later
>> +
>> +import logging
>> +
>> +from .machine import QEMUMachine
>> +
>> +LOG = logging.getLogger(__name__)
>> +
>> +
>> +def binary_get_version(qemu_bin):
>> +    '''
>> +    Get QEMU binary version
>> +
>> +    @param qemu_bin (str): path to the QEMU binary
> 
> It could check that qemu_bin file exists, otherwise raise an exception 
> or return None.

OK.

> 
>> +    @return binary version (dictionary with major/minor/micro keys)
>> +    '''
>> +    with QEMUMachine(qemu_bin) as vm:
>> +        vm.set_machine('none')
>> +        vm.launch()
>> +        res = vm.command('query-version')
>> +        LOG.info(res)
>> +        vm.shutdown()
> 
> Don't need this, the vm will be shutdown anyway (see 
> QEMUMachine.__exit__())

OK.

> 
> Thanks!
> 
> - Wainer
> 
> 
>> +        return res['qemu']
>> diff --git a/tests/acceptance/core_scripts.py 
>> b/tests/acceptance/core_scripts.py
>> new file mode 100644
>> index 0000000000..3f253337cd
>> --- /dev/null
>> +++ b/tests/acceptance/core_scripts.py
>> @@ -0,0 +1,31 @@
>> +# Tests covering various python/qemu/ scripts
>> +#
>> +# Copyright (c) 2020 Red Hat, Inc.
>> +#
>> +# Author:
>> +#  Philippe Mathieu-Daudé <philmd@redhat.com>
>> +#
>> +# This work is licensed under the terms of the GNU GPL, version 2 or 
>> later.
>> +# See the COPYING file in the top-level directory.
>> +#
>> +# SPDX-License-Identifier: GPL-2.0-or-later
>> +
>> +import sys
>> +import os
>> +import logging
>> +
>> +sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 
>> 'python'))
>> +from avocado_qemu import Test
>> +from qemu.binutils import binary_get_version
>> +
>> +
>> +class PythonQemuCoreScripts(Test):
>> +
>> +    def test_get_version(self):
>> +        logger = logging.getLogger('core')
>> +        version = binary_get_version(self.qemu_bin)
>> +        logger.debug('version: {}'.format(version))
>> +        # QMP 'query-version' introduced with QEMU v0.14
>> +        self.assertGreaterEqual(version['major'], 0)
>> +        if version['major'] == 0:
>> +            self.assertGreaterEqual(version['minor'], 14)
>