From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58412) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gAELz-0007Nx-QL for qemu-devel@nongnu.org; Wed, 10 Oct 2018 09:16:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gAELs-0008JM-BY for qemu-devel@nongnu.org; Wed, 10 Oct 2018 09:16:40 -0400 Received: from mx1.redhat.com ([209.132.183.28]:41766) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gAELr-0008Iu-Uv for qemu-devel@nongnu.org; Wed, 10 Oct 2018 09:16:36 -0400 References: <20181004151429.7232-1-crosa@redhat.com> <20181004151429.7232-3-crosa@redhat.com> <20181004235602.GA24920@kermit-br-ibm-com> From: Cleber Rosa Message-ID: Date: Wed, 10 Oct 2018 09:16:22 -0400 MIME-Version: 1.0 In-Reply-To: <20181004235602.GA24920@kermit-br-ibm-com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/7] Acceptance Tests: introduce arch parameter and attribute List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Murilo Opsfelder Araujo Cc: qemu-devel@nongnu.org, Fam Zheng , Eduardo Habkost , Laszlo Ersek , =?UTF-8?Q?Philippe_Mathieu-Daud=c3=a9?= , Stefan Hajnoczi , Caio Carrara , =?UTF-8?Q?Alex_Benn=c3=a9e?= On 10/4/18 7:56 PM, Murilo Opsfelder Araujo wrote: > Hi, Cleber. > > On Thu, Oct 04, 2018 at 11:14:24AM -0400, Cleber Rosa wrote: >> On a number of different scenarios, such as when choosing a QEMU >> binary to be used on tests (or a image to use to boot a test VM), it's >> useful to define the architecture that should be used. >> >> This introduces both a test parameter and a test instance attribute, >> that will contain such a value. >> >> The selection of the default QEMU binary, if one is not given >> explicitly, has also been changed to look at the arch attribute. >> >> Signed-off-by: Cleber Rosa >> --- >> docs/devel/testing.rst | 18 ++++++++++++++++++ >> tests/acceptance/avocado_qemu/__init__.py | 13 ++++++++++--- >> 2 files changed, 28 insertions(+), 3 deletions(-) >> >> diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst >> index 727c4019b5..4178ae5022 100644 >> --- a/docs/devel/testing.rst >> +++ b/docs/devel/testing.rst >> @@ -659,6 +659,17 @@ vm >> A QEMUMachine instance, initially configured according to the given >> ``qemu_bin`` parameter. >> >> +arch >> +~~~~ >> + >> +The architecture that will be used on a number of different >> +scenarios. For instance, when a QEMU binary is not explicitly given, >> +the one selected will depend on this attribute. >> + >> +The ``arch`` attribute will be set to the test parameter of the same >> +name, and if one is not given explicitly, it will default to the >> +system architecture (as given by ``uname``). >> + >> qemu_bin >> ~~~~~~~~ >> >> @@ -681,6 +692,13 @@ like the following: >> >> PARAMS (key=qemu_bin, path=*, default=x86_64-softmmu/qemu-system-x86_64) => 'x86_64-softmmu/qemu-system-x86_64 >> >> +arch >> +~~~~ >> + >> +The architecture that will be used on a number of different scenarios. >> +This parameter has a direct relation with the ``arch`` attribute. If >> +not given, it will default to None. >> + >> qemu_bin >> ~~~~~~~~ >> >> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py >> index d8d5b48dac..9329d9b9ec 100644 >> --- a/tests/acceptance/avocado_qemu/__init__.py >> +++ b/tests/acceptance/avocado_qemu/__init__.py >> @@ -23,16 +23,22 @@ def is_readable_executable_file(path): >> return os.path.isfile(path) and os.access(path, os.R_OK | os.X_OK) >> >> >> -def pick_default_qemu_bin(): >> +def pick_default_qemu_bin(arch=None): >> """ >> Picks the path of a QEMU binary, starting either in the current working >> directory or in the source tree root directory. >> >> + :param arch: the arch to use when looking for a QEMU binary (the target >> + will match the arch given). If None (the default) arch >> + will be the current host system arch (as given by >> + :func:`os.uname`). >> + :param arch: str > > Do you mean > > :type arch: str > > ? > Hi Murilo, Yes, thanks for catching that. >> :returns: the path to the default QEMU binary or None if one could not >> be found >> :rtype: str or None >> """ >> - arch = os.uname()[4] >> + if arch is None: >> + arch = os.uname()[4] >> qemu_bin_relative_path = os.path.join("%s-softmmu" % arch, >> "qemu-system-%s" % arch) > > On ppc64le systems, this can lead to an unexisting path. > > For instance, os.uname() returns: > > >>> os.uname() > ('Linux', 'localhost', '4.15.0-34-generic', '#37-Ubuntu SMP Mon Aug 27 15:18:58 UTC 2018', 'ppc64le') > > Then, qemu_bin_relative_path would be set to: > > ppc64le-softmmu/qemu-system-ppc64le > > However, the correct path is: > > ppc64-softmmu/qemu-system-ppc64 > > I'm not sure if ppc64le is the only case where the target name is different from > the OS architecture. > > If you decide not to handle this now, at least add a FIXME, if possible. > You're correct, but this behavior is also unrelated to the *this* specific change (the uname() based arch was already being used). Because of that I'd rather send the fix in another series. To track the fix I've created the following card: https://trello.com/c/0quVUfKi/48-support-target-binary-selection-on-ppc64le Thanks! - Cleber. >> if is_readable_executable_file(qemu_bin_relative_path): >> @@ -47,8 +53,9 @@ def pick_default_qemu_bin(): >> class Test(avocado.Test): >> def setUp(self): >> self.vm = None >> + self.arch = self.params.get('arch', default=os.uname()[4]) >> self.qemu_bin = self.params.get('qemu_bin', >> - default=pick_default_qemu_bin()) >> + default=pick_default_qemu_bin(self.arch)) >> if self.qemu_bin is None: >> self.cancel("No QEMU binary defined or found in the source tree") >> self.vm = QEMUMachine(self.qemu_bin) >> -- >> 2.17.1 >> >> > > -- > Murilo >