| Message ID | 20190202005610.24048-6-crosa@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Acceptance Tests: target architecture support | expand |
On Fri, 1 Feb 2019 19:55:55 -0500 Cleber Rosa <crosa@redhat.com> wrote: > It's useful to define the architecture that should be used in > situations such as: > * the intended target of the QEMU binary to be used on tests > * the architecture of code to be run within the QEMU binary, such > as a kernel image or a full blown guest OS image Thinking a bit more about this: These two are often, but not necessarily, the same. For example, starting a machine with the 64 bit variant of an architecture to run a guest with the 32 bit variant of that architecture might be a valid case. > > This commit introduces both a test parameter and a test instance > attribute, that will contain such a value. > > Now, when the "arch" test parameter is given, it will influence the > selection of the default QEMU binary, if one is not given explicitly > by means of the "qemu_img" parameter. > > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > Signed-off-by: Cleber Rosa <crosa@redhat.com> > --- > docs/devel/testing.rst | 17 +++++++++++++++++ > tests/acceptance/avocado_qemu/__init__.py | 14 +++++++++++--- > 2 files changed, 28 insertions(+), 3 deletions(-) > > diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst > index 44c9b3ae74..d37c4b0e77 100644 > --- a/docs/devel/testing.rst > +++ b/docs/devel/testing.rst > @@ -689,6 +689,16 @@ 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. This is probably a bit too vague. (What are "different scenarios"?) Does it select anything else than the architecture of the vm that will be started? > + > +The ``arch`` attribute will be set to the test parameter of the same > +name, and if one is not given explicitly, it will be set to ``None``. > + > qemu_bin > ~~~~~~~~ > > @@ -711,6 +721,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. Same comment here. > +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..f580582602 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`). This is more specific :) > + :type arch: str > :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) > if is_readable_executable_file(qemu_bin_relative_path): > @@ -47,8 +53,10 @@ def pick_default_qemu_bin(): > class Test(avocado.Test): > def setUp(self): > self.vm = None > + self.arch = self.params.get('arch') > + default_qemu_bin = pick_default_qemu_bin(arch=self.arch) > self.qemu_bin = self.params.get('qemu_bin', > - default=pick_default_qemu_bin()) > + default=default_qemu_bin) > if self.qemu_bin is None: > self.cancel("No QEMU binary defined or found in the source tree") > self.vm = QEMUMachine(self.qemu_bin)
On 2/6/19 10:40 AM, Cornelia Huck wrote: > On Fri, 1 Feb 2019 19:55:55 -0500 > Cleber Rosa <crosa@redhat.com> wrote: > >> It's useful to define the architecture that should be used in >> situations such as: >> * the intended target of the QEMU binary to be used on tests >> * the architecture of code to be run within the QEMU binary, such >> as a kernel image or a full blown guest OS image > > Thinking a bit more about this: These two are often, but not > necessarily, the same. For example, starting a machine with the 64 bit > variant of an architecture to run a guest with the 32 bit variant of > that architecture might be a valid case. > I agree with everything you said, and that's why I imagine "arch" being used as a safe default type of parameter. See, the QEMU binary can be influenced by the "arch" parameter, but can still be *defined* by the "qemu_bin" parameter. That's the same approach I believe can be applied to the architecture of the guest code. When time comes, we may add a "guest_arch" parameter of sorts. We don't have that use case now, but I believe we'll be covered when it comes. >> >> This commit introduces both a test parameter and a test instance >> attribute, that will contain such a value. >> >> Now, when the "arch" test parameter is given, it will influence the >> selection of the default QEMU binary, if one is not given explicitly >> by means of the "qemu_img" parameter. >> >> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> Signed-off-by: Cleber Rosa <crosa@redhat.com> >> --- >> docs/devel/testing.rst | 17 +++++++++++++++++ >> tests/acceptance/avocado_qemu/__init__.py | 14 +++++++++++--- >> 2 files changed, 28 insertions(+), 3 deletions(-) >> >> diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst >> index 44c9b3ae74..d37c4b0e77 100644 >> --- a/docs/devel/testing.rst >> +++ b/docs/devel/testing.rst >> @@ -689,6 +689,16 @@ 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. > > This is probably a bit too vague. (What are "different scenarios"?) > I believe it's kind of vague because I was thinking of both the "framework level" code, and the possible uses in the tests. At the "framework" level, it's only used for the case listed there ("when a QEMU binary is not explicitly given..."). > Does it select anything else than the architecture of the vm that will > be started? > No, it doesn't. The idea here was that tests may choose to use the same attribute value in their own specific ("different") scenarios. I can certainly make this clearer. >> + >> +The ``arch`` attribute will be set to the test parameter of the same >> +name, and if one is not given explicitly, it will be set to ``None``. >> + >> qemu_bin >> ~~~~~~~~ >> >> @@ -711,6 +721,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. > > Same comment here. > ACK. >> +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..f580582602 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`). > > This is more specific :) > Yep, I'll make the other doc snippets clearer. Thanks! - Cleber. >> + :type arch: str >> :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) >> if is_readable_executable_file(qemu_bin_relative_path): >> @@ -47,8 +53,10 @@ def pick_default_qemu_bin(): >> class Test(avocado.Test): >> def setUp(self): >> self.vm = None >> + self.arch = self.params.get('arch') >> + default_qemu_bin = pick_default_qemu_bin(arch=self.arch) >> self.qemu_bin = self.params.get('qemu_bin', >> - default=pick_default_qemu_bin()) >> + default=default_qemu_bin) >> if self.qemu_bin is None: >> self.cancel("No QEMU binary defined or found in the source tree") >> self.vm = QEMUMachine(self.qemu_bin) >
On 2/7/19 1:02 PM, Cleber Rosa wrote: > > > On 2/6/19 10:40 AM, Cornelia Huck wrote: >> On Fri, 1 Feb 2019 19:55:55 -0500 >> Cleber Rosa <crosa@redhat.com> wrote: >> >>> It's useful to define the architecture that should be used in >>> situations such as: >>> * the intended target of the QEMU binary to be used on tests >>> * the architecture of code to be run within the QEMU binary, such >>> as a kernel image or a full blown guest OS image >> >> Thinking a bit more about this: These two are often, but not >> necessarily, the same. For example, starting a machine with the 64 bit >> variant of an architecture to run a guest with the 32 bit variant of >> that architecture might be a valid case. >> > > I agree with everything you said, and that's why I imagine "arch" being > used as a safe default type of parameter. > > See, the QEMU binary can be influenced by the "arch" parameter, but can > still be *defined* by the "qemu_bin" parameter. That's the same > approach I believe can be applied to the architecture of the guest code. > When time comes, we may add a "guest_arch" parameter of sorts. We > don't have that use case now, but I believe we'll be covered when it comes. > >>> >>> This commit introduces both a test parameter and a test instance >>> attribute, that will contain such a value. >>> >>> Now, when the "arch" test parameter is given, it will influence the >>> selection of the default QEMU binary, if one is not given explicitly >>> by means of the "qemu_img" parameter. >>> >>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> >>> Signed-off-by: Cleber Rosa <crosa@redhat.com> >>> --- >>> docs/devel/testing.rst | 17 +++++++++++++++++ >>> tests/acceptance/avocado_qemu/__init__.py | 14 +++++++++++--- >>> 2 files changed, 28 insertions(+), 3 deletions(-) >>> >>> diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst >>> index 44c9b3ae74..d37c4b0e77 100644 >>> --- a/docs/devel/testing.rst >>> +++ b/docs/devel/testing.rst >>> @@ -689,6 +689,16 @@ 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. >> >> This is probably a bit too vague. (What are "different scenarios"?) >> > > I believe it's kind of vague because I was thinking of both the > "framework level" code, and the possible uses in the tests. > > At the "framework" level, it's only used for the case listed there > ("when a QEMU binary is not explicitly given..."). > >> Does it select anything else than the architecture of the vm that will >> be started? >> > > No, it doesn't. The idea here was that tests may choose to use the same > attribute value in their own specific ("different") scenarios. > > I can certainly make this clearer. > Hi Cornelia, How does this sound? --- The architecture that will influence the selection of a QEMU binary (when one is not explicitly given). Tests are also free to use this attribute value, for their own needs. A test may, for instance, use the same value when selecting the architecture of a kernel or disk image to boot a VM with. The ``arch`` attribute will be set to the test parameter of the same name, and if one is not given explicitly, it will be set to ``None``. --- It uses your previous point about the 64/32 bit host/guest as an example. Again, a test could use another parameter (not a Test class attribute) if it wants to use code for a different arch than the host. Thanks, - Cleber. >>> + >>> +The ``arch`` attribute will be set to the test parameter of the same >>> +name, and if one is not given explicitly, it will be set to ``None``. >>> + >>> qemu_bin >>> ~~~~~~~~ >>> >>> @@ -711,6 +721,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. >> >> Same comment here. >> > > ACK. > >>> +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..f580582602 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`). >> >> This is more specific :) >> > > Yep, I'll make the other doc snippets clearer. > > Thanks! > - Cleber. > >>> + :type arch: str >>> :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) >>> if is_readable_executable_file(qemu_bin_relative_path): >>> @@ -47,8 +53,10 @@ def pick_default_qemu_bin(): >>> class Test(avocado.Test): >>> def setUp(self): >>> self.vm = None >>> + self.arch = self.params.get('arch') >>> + default_qemu_bin = pick_default_qemu_bin(arch=self.arch) >>> self.qemu_bin = self.params.get('qemu_bin', >>> - default=pick_default_qemu_bin()) >>> + default=default_qemu_bin) >>> if self.qemu_bin is None: >>> self.cancel("No QEMU binary defined or found in the source tree") >>> self.vm = QEMUMachine(self.qemu_bin) >>
On Thu, 7 Feb 2019 13:22:10 -0500 Cleber Rosa <crosa@redhat.com> wrote: > On 2/7/19 1:02 PM, Cleber Rosa wrote: > > > > > > On 2/6/19 10:40 AM, Cornelia Huck wrote: > >> On Fri, 1 Feb 2019 19:55:55 -0500 > >> Cleber Rosa <crosa@redhat.com> wrote: > >> > >>> It's useful to define the architecture that should be used in > >>> situations such as: > >>> * the intended target of the QEMU binary to be used on tests > >>> * the architecture of code to be run within the QEMU binary, such > >>> as a kernel image or a full blown guest OS image > >> > >> Thinking a bit more about this: These two are often, but not > >> necessarily, the same. For example, starting a machine with the 64 bit > >> variant of an architecture to run a guest with the 32 bit variant of > >> that architecture might be a valid case. > >> > > > > I agree with everything you said, and that's why I imagine "arch" being > > used as a safe default type of parameter. > > > > See, the QEMU binary can be influenced by the "arch" parameter, but can > > still be *defined* by the "qemu_bin" parameter. That's the same > > approach I believe can be applied to the architecture of the guest code. > > When time comes, we may add a "guest_arch" parameter of sorts. We > > don't have that use case now, but I believe we'll be covered when it comes. Ok, makes sense. > > > >>> > >>> This commit introduces both a test parameter and a test instance > >>> attribute, that will contain such a value. > >>> > >>> Now, when the "arch" test parameter is given, it will influence the > >>> selection of the default QEMU binary, if one is not given explicitly > >>> by means of the "qemu_img" parameter. > >>> > >>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > >>> Signed-off-by: Cleber Rosa <crosa@redhat.com> > >>> --- > >>> docs/devel/testing.rst | 17 +++++++++++++++++ > >>> tests/acceptance/avocado_qemu/__init__.py | 14 +++++++++++--- > >>> 2 files changed, 28 insertions(+), 3 deletions(-) > >>> > >>> diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst > >>> index 44c9b3ae74..d37c4b0e77 100644 > >>> --- a/docs/devel/testing.rst > >>> +++ b/docs/devel/testing.rst > >>> @@ -689,6 +689,16 @@ 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. > >> > >> This is probably a bit too vague. (What are "different scenarios"?) > >> > > > > I believe it's kind of vague because I was thinking of both the > > "framework level" code, and the possible uses in the tests. > > > > At the "framework" level, it's only used for the case listed there > > ("when a QEMU binary is not explicitly given..."). > > > >> Does it select anything else than the architecture of the vm that will > >> be started? > >> > > > > No, it doesn't. The idea here was that tests may choose to use the same > > attribute value in their own specific ("different") scenarios. > > > > I can certainly make this clearer. > > > > Hi Cornelia, > > How does this sound? > > --- What about adding the following as a lead-in: "The architecture can be used on different levels of the stack, e.g. by the framework or by the test itself." That would probably explain what you wanted to express with "different scenarios". > > The architecture that will influence the selection of a QEMU binary > (when one is not explicitly given). s/that will/will/ ? > > Tests are also free to use this attribute value, for their own needs. > A test may, for instance, use the same value when selecting the > architecture of a kernel or disk image to boot a VM with. > > The ``arch`` attribute will be set to the test parameter of the same > name, and if one is not given explicitly, it will be set to ``None``. > > --- > > It uses your previous point about the 64/32 bit host/guest as an > example. Again, a test could use another parameter (not a Test class > attribute) if it wants to use code for a different arch than the host. Sounds good!
diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst index 44c9b3ae74..d37c4b0e77 100644 --- a/docs/devel/testing.rst +++ b/docs/devel/testing.rst @@ -689,6 +689,16 @@ 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 be set to ``None``. + qemu_bin ~~~~~~~~ @@ -711,6 +721,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..f580582602 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`). + :type arch: str :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) if is_readable_executable_file(qemu_bin_relative_path): @@ -47,8 +53,10 @@ def pick_default_qemu_bin(): class Test(avocado.Test): def setUp(self): self.vm = None + self.arch = self.params.get('arch') + default_qemu_bin = pick_default_qemu_bin(arch=self.arch) self.qemu_bin = self.params.get('qemu_bin', - default=pick_default_qemu_bin()) + default=default_qemu_bin) if self.qemu_bin is None: self.cancel("No QEMU binary defined or found in the source tree") self.vm = QEMUMachine(self.qemu_bin)