| Message ID | 20181009232607.15521-5-crosa@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Acceptance Tests: basic architecture support | expand |
On 10/10/2018 01:26, Cleber Rosa wrote: > Some targets require a machine type to be set, as there's no default > (aarch64 is one example). To give a consistent interface to users of > this API, this changes set_machine() so that a predefined default can > be used, if one is not given. The approach used is exactly the same > with the console device type. > > Also, even when there's a default machine type, for some purposes, > testing included, it's better if outside code is explicit about the > machine type, instead of relying on whatever is set internally. > > Signed-off-by: Cleber Rosa <crosa@redhat.com> > --- > scripts/qemu.py | 22 +++++++++++++++++++++- > 1 file changed, 21 insertions(+), 1 deletion(-) > > diff --git a/scripts/qemu.py b/scripts/qemu.py > index d9e24a0c1a..fca9b76990 100644 > --- a/scripts/qemu.py > +++ b/scripts/qemu.py > @@ -36,6 +36,15 @@ CONSOLE_DEV_TYPES = { > r'^s390-ccw-virtio.*': 'sclpconsole', > } > > +#: Maps archictures to the preferred machine type > +MACHINE_TYPES = { > + r'^aarch64$': 'virt', > + r'^ppc$': 'g3beige', > + r'^ppc64$': 'pseries', > + r'^s390x$': 's390-ccw-virtio', > + r'^x86_64$': 'q35', Why choose Q35 rather than PC (the default)? I was wondering about how to generate variants/machines.json but this is definitively something we want to do via a QMP query. Eduardo what do you think? > + } > + > > class QEMUMachineError(Exception): > """ > @@ -413,13 +422,24 @@ class QEMUMachine(object): > """ > self._arch = arch > > - def set_machine(self, machine_type): > + def set_machine(self, machine_type=None): > ''' > Sets the machine type > > If set, the machine type will be added to the base arguments > of the resulting QEMU command line. > ''' > + if machine_type is None: > + if self._arch is None: > + raise QEMUMachineError("Can not set a default machine type: " > + "QEMU instance without a defined arch") > + for regex, machine in MACHINE_TYPES.items(): > + if re.match(regex, self._arch): > + machine_type = machine > + break > + if machine_type is None: > + raise QEMUMachineError("Can not set a machine type: no " > + "matching machine type definition") > self._machine = machine_type > > def set_console(self, device_type=None): >
On 10/10/18 7:00 AM, Philippe Mathieu-Daudé wrote: > On 10/10/2018 01:26, Cleber Rosa wrote: >> Some targets require a machine type to be set, as there's no default >> (aarch64 is one example). To give a consistent interface to users of >> this API, this changes set_machine() so that a predefined default can >> be used, if one is not given. The approach used is exactly the same >> with the console device type. >> >> Also, even when there's a default machine type, for some purposes, >> testing included, it's better if outside code is explicit about the >> machine type, instead of relying on whatever is set internally. >> >> Signed-off-by: Cleber Rosa <crosa@redhat.com> >> --- >> scripts/qemu.py | 22 +++++++++++++++++++++- >> 1 file changed, 21 insertions(+), 1 deletion(-) >> >> diff --git a/scripts/qemu.py b/scripts/qemu.py >> index d9e24a0c1a..fca9b76990 100644 >> --- a/scripts/qemu.py >> +++ b/scripts/qemu.py >> @@ -36,6 +36,15 @@ CONSOLE_DEV_TYPES = { >> r'^s390-ccw-virtio.*': 'sclpconsole', >> } >> >> +#: Maps archictures to the preferred machine type >> +MACHINE_TYPES = { >> + r'^aarch64$': 'virt', >> + r'^ppc$': 'g3beige', >> + r'^ppc64$': 'pseries', >> + r'^s390x$': 's390-ccw-virtio', >> + r'^x86_64$': 'q35', > > Why choose Q35 rather than PC (the default)? > > I was wondering about how to generate variants/machines.json but this is > definitively something we want to do via a QMP query. > > Eduardo what do you think? > It was motivated by Eduardo's initiative to make q35 the default "across the board". He can confirm and give more details. - Cleber. >> + } >> + >> >> class QEMUMachineError(Exception): >> """ >> @@ -413,13 +422,24 @@ class QEMUMachine(object): >> """ >> self._arch = arch >> >> - def set_machine(self, machine_type): >> + def set_machine(self, machine_type=None): >> ''' >> Sets the machine type >> >> If set, the machine type will be added to the base arguments >> of the resulting QEMU command line. >> ''' >> + if machine_type is None: >> + if self._arch is None: >> + raise QEMUMachineError("Can not set a default machine type: " >> + "QEMU instance without a defined arch") >> + for regex, machine in MACHINE_TYPES.items(): >> + if re.match(regex, self._arch): >> + machine_type = machine >> + break >> + if machine_type is None: >> + raise QEMUMachineError("Can not set a machine type: no " >> + "matching machine type definition") >> self._machine = machine_type >> >> def set_console(self, device_type=None): >>
On Wed, Oct 10, 2018 at 08:35:38AM -0400, Cleber Rosa wrote: > > > On 10/10/18 7:00 AM, Philippe Mathieu-Daudé wrote: > > On 10/10/2018 01:26, Cleber Rosa wrote: > >> Some targets require a machine type to be set, as there's no default > >> (aarch64 is one example). To give a consistent interface to users of > >> this API, this changes set_machine() so that a predefined default can > >> be used, if one is not given. The approach used is exactly the same > >> with the console device type. > >> > >> Also, even when there's a default machine type, for some purposes, > >> testing included, it's better if outside code is explicit about the > >> machine type, instead of relying on whatever is set internally. > >> > >> Signed-off-by: Cleber Rosa <crosa@redhat.com> > >> --- > >> scripts/qemu.py | 22 +++++++++++++++++++++- > >> 1 file changed, 21 insertions(+), 1 deletion(-) > >> > >> diff --git a/scripts/qemu.py b/scripts/qemu.py > >> index d9e24a0c1a..fca9b76990 100644 > >> --- a/scripts/qemu.py > >> +++ b/scripts/qemu.py > >> @@ -36,6 +36,15 @@ CONSOLE_DEV_TYPES = { > >> r'^s390-ccw-virtio.*': 'sclpconsole', > >> } > >> > >> +#: Maps archictures to the preferred machine type > >> +MACHINE_TYPES = { > >> + r'^aarch64$': 'virt', > >> + r'^ppc$': 'g3beige', > >> + r'^ppc64$': 'pseries', > >> + r'^s390x$': 's390-ccw-virtio', > >> + r'^x86_64$': 'q35', > > > > Why choose Q35 rather than PC (the default)? > > > > I was wondering about how to generate variants/machines.json but this is > > definitively something we want to do via a QMP query. > > > > Eduardo what do you think? > > > > It was motivated by Eduardo's initiative to make q35 the default "across > the board". He can confirm and give more details. Making Q35 the default on applications using QEMU and libvirt is something I'd like to happen. But I think the simplest way to do that is to change the QEMU default. This way you won't need this table on qemu.py: you can just use the default provided by QEMU.
On 10/10/18 9:46 AM, Eduardo Habkost wrote: > On Wed, Oct 10, 2018 at 08:35:38AM -0400, Cleber Rosa wrote: >> >> >> On 10/10/18 7:00 AM, Philippe Mathieu-Daudé wrote: >>> On 10/10/2018 01:26, Cleber Rosa wrote: >>>> Some targets require a machine type to be set, as there's no default >>>> (aarch64 is one example). To give a consistent interface to users of >>>> this API, this changes set_machine() so that a predefined default can >>>> be used, if one is not given. The approach used is exactly the same >>>> with the console device type. >>>> >>>> Also, even when there's a default machine type, for some purposes, >>>> testing included, it's better if outside code is explicit about the >>>> machine type, instead of relying on whatever is set internally. >>>> >>>> Signed-off-by: Cleber Rosa <crosa@redhat.com> >>>> --- >>>> scripts/qemu.py | 22 +++++++++++++++++++++- >>>> 1 file changed, 21 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/scripts/qemu.py b/scripts/qemu.py >>>> index d9e24a0c1a..fca9b76990 100644 >>>> --- a/scripts/qemu.py >>>> +++ b/scripts/qemu.py >>>> @@ -36,6 +36,15 @@ CONSOLE_DEV_TYPES = { >>>> r'^s390-ccw-virtio.*': 'sclpconsole', >>>> } >>>> >>>> +#: Maps archictures to the preferred machine type >>>> +MACHINE_TYPES = { >>>> + r'^aarch64$': 'virt', >>>> + r'^ppc$': 'g3beige', >>>> + r'^ppc64$': 'pseries', >>>> + r'^s390x$': 's390-ccw-virtio', >>>> + r'^x86_64$': 'q35', >>> >>> Why choose Q35 rather than PC (the default)? >>> >>> I was wondering about how to generate variants/machines.json but this is >>> definitively something we want to do via a QMP query. >>> >>> Eduardo what do you think? >>> >> >> It was motivated by Eduardo's initiative to make q35 the default "across >> the board". He can confirm and give more details. > > Making Q35 the default on applications using QEMU and libvirt is > something I'd like to happen. But I think the simplest way to do > that is to change the QEMU default. This way you won't need this > table on qemu.py: you can just use the default provided by QEMU. > The idea is to bring consistency on how we're calling "qemu-system-$(ARCH)", and at the same time apply the "explicit is better than implicit" rule. The most important fact is that some targets do not (currently) have "the default provided by QEMU", aarch64 is one of them. - Cleber.
On 10/10/18 9:59 AM, Cleber Rosa wrote: > > > On 10/10/18 9:46 AM, Eduardo Habkost wrote: >> On Wed, Oct 10, 2018 at 08:35:38AM -0400, Cleber Rosa wrote: >>> >>> >>> On 10/10/18 7:00 AM, Philippe Mathieu-Daudé wrote: >>>> On 10/10/2018 01:26, Cleber Rosa wrote: >>>>> Some targets require a machine type to be set, as there's no default >>>>> (aarch64 is one example). To give a consistent interface to users of >>>>> this API, this changes set_machine() so that a predefined default can >>>>> be used, if one is not given. The approach used is exactly the same >>>>> with the console device type. >>>>> >>>>> Also, even when there's a default machine type, for some purposes, >>>>> testing included, it's better if outside code is explicit about the >>>>> machine type, instead of relying on whatever is set internally. >>>>> >>>>> Signed-off-by: Cleber Rosa <crosa@redhat.com> >>>>> --- >>>>> scripts/qemu.py | 22 +++++++++++++++++++++- >>>>> 1 file changed, 21 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/scripts/qemu.py b/scripts/qemu.py >>>>> index d9e24a0c1a..fca9b76990 100644 >>>>> --- a/scripts/qemu.py >>>>> +++ b/scripts/qemu.py >>>>> @@ -36,6 +36,15 @@ CONSOLE_DEV_TYPES = { >>>>> r'^s390-ccw-virtio.*': 'sclpconsole', >>>>> } >>>>> >>>>> +#: Maps archictures to the preferred machine type >>>>> +MACHINE_TYPES = { >>>>> + r'^aarch64$': 'virt', >>>>> + r'^ppc$': 'g3beige', >>>>> + r'^ppc64$': 'pseries', >>>>> + r'^s390x$': 's390-ccw-virtio', >>>>> + r'^x86_64$': 'q35', >>>> >>>> Why choose Q35 rather than PC (the default)? >>>> >>>> I was wondering about how to generate variants/machines.json but this is >>>> definitively something we want to do via a QMP query. >>>> >>>> Eduardo what do you think? >>>> >>> >>> It was motivated by Eduardo's initiative to make q35 the default "across >>> the board". He can confirm and give more details. >> >> Making Q35 the default on applications using QEMU and libvirt is >> something I'd like to happen. But I think the simplest way to do >> that is to change the QEMU default. This way you won't need this >> table on qemu.py: you can just use the default provided by QEMU. >> > > The idea is to bring consistency on how we're calling > "qemu-system-$(ARCH)", and at the same time apply the "explicit is > better than implicit" rule. > > The most important fact is that some targets do not (currently) have > "the default provided by QEMU", aarch64 is one of them. > > - Cleber. > So I ended up not relaying the question properly: should we default (even if explicitly adding "-machine") to "pc"? Thanks! - Cleber.
On Wed, Oct 10, 2018 at 10:15:15AM -0400, Cleber Rosa wrote: > > > On 10/10/18 9:59 AM, Cleber Rosa wrote: > > > > > > On 10/10/18 9:46 AM, Eduardo Habkost wrote: > >> On Wed, Oct 10, 2018 at 08:35:38AM -0400, Cleber Rosa wrote: > >>> > >>> > >>> On 10/10/18 7:00 AM, Philippe Mathieu-Daudé wrote: > >>>> On 10/10/2018 01:26, Cleber Rosa wrote: > >>>>> Some targets require a machine type to be set, as there's no default > >>>>> (aarch64 is one example). To give a consistent interface to users of > >>>>> this API, this changes set_machine() so that a predefined default can > >>>>> be used, if one is not given. The approach used is exactly the same > >>>>> with the console device type. > >>>>> > >>>>> Also, even when there's a default machine type, for some purposes, > >>>>> testing included, it's better if outside code is explicit about the > >>>>> machine type, instead of relying on whatever is set internally. > >>>>> > >>>>> Signed-off-by: Cleber Rosa <crosa@redhat.com> > >>>>> --- > >>>>> scripts/qemu.py | 22 +++++++++++++++++++++- > >>>>> 1 file changed, 21 insertions(+), 1 deletion(-) > >>>>> > >>>>> diff --git a/scripts/qemu.py b/scripts/qemu.py > >>>>> index d9e24a0c1a..fca9b76990 100644 > >>>>> --- a/scripts/qemu.py > >>>>> +++ b/scripts/qemu.py > >>>>> @@ -36,6 +36,15 @@ CONSOLE_DEV_TYPES = { > >>>>> r'^s390-ccw-virtio.*': 'sclpconsole', > >>>>> } > >>>>> > >>>>> +#: Maps archictures to the preferred machine type > >>>>> +MACHINE_TYPES = { > >>>>> + r'^aarch64$': 'virt', > >>>>> + r'^ppc$': 'g3beige', > >>>>> + r'^ppc64$': 'pseries', > >>>>> + r'^s390x$': 's390-ccw-virtio', > >>>>> + r'^x86_64$': 'q35', > >>>> > >>>> Why choose Q35 rather than PC (the default)? > >>>> > >>>> I was wondering about how to generate variants/machines.json but this is > >>>> definitively something we want to do via a QMP query. > >>>> > >>>> Eduardo what do you think? > >>>> > >>> > >>> It was motivated by Eduardo's initiative to make q35 the default "across > >>> the board". He can confirm and give more details. > >> > >> Making Q35 the default on applications using QEMU and libvirt is > >> something I'd like to happen. But I think the simplest way to do > >> that is to change the QEMU default. This way you won't need this > >> table on qemu.py: you can just use the default provided by QEMU. > >> > > > > The idea is to bring consistency on how we're calling > > "qemu-system-$(ARCH)", and at the same time apply the "explicit is > > better than implicit" rule. > > > > The most important fact is that some targets do not (currently) have > > "the default provided by QEMU", aarch64 is one of them. > > > > - Cleber. > > > > So I ended up not relaying the question properly: should we default > (even if explicitly adding "-machine") to "pc"? I think using the default machine-type (when QEMU has a default) would be less surprising for users of the qemu.py API. Implicitly adding -machine when there's no default is also surprising, but then it's a nice surprise: instead of crashing you get a running VM. Now, there are two other questions related to this: If using 'pc' as default, should we always add -machine, or just omit the machine-type name? I think we should omit it unless the caller asked for a specific machine-type name (because it would be less surprising for users of the API). About our default testing configuration for acceptance tests: should acceptance tests run against PC by default? Should it test Q35? Should we test both PC and Q35? I'm not sure what's the answer, but I think these decisions shouldn't affect the qemu.py API at all.
On 10/10/2018 16:28, Eduardo Habkost wrote: > On Wed, Oct 10, 2018 at 10:15:15AM -0400, Cleber Rosa wrote: >> >> >> On 10/10/18 9:59 AM, Cleber Rosa wrote: >>> >>> >>> On 10/10/18 9:46 AM, Eduardo Habkost wrote: >>>> On Wed, Oct 10, 2018 at 08:35:38AM -0400, Cleber Rosa wrote: >>>>> >>>>> >>>>> On 10/10/18 7:00 AM, Philippe Mathieu-Daudé wrote: >>>>>> On 10/10/2018 01:26, Cleber Rosa wrote: >>>>>>> Some targets require a machine type to be set, as there's no default >>>>>>> (aarch64 is one example). To give a consistent interface to users of >>>>>>> this API, this changes set_machine() so that a predefined default can >>>>>>> be used, if one is not given. The approach used is exactly the same >>>>>>> with the console device type. >>>>>>> >>>>>>> Also, even when there's a default machine type, for some purposes, >>>>>>> testing included, it's better if outside code is explicit about the >>>>>>> machine type, instead of relying on whatever is set internally. >>>>>>> >>>>>>> Signed-off-by: Cleber Rosa <crosa@redhat.com> >>>>>>> --- >>>>>>> scripts/qemu.py | 22 +++++++++++++++++++++- >>>>>>> 1 file changed, 21 insertions(+), 1 deletion(-) >>>>>>> >>>>>>> diff --git a/scripts/qemu.py b/scripts/qemu.py >>>>>>> index d9e24a0c1a..fca9b76990 100644 >>>>>>> --- a/scripts/qemu.py >>>>>>> +++ b/scripts/qemu.py >>>>>>> @@ -36,6 +36,15 @@ CONSOLE_DEV_TYPES = { >>>>>>> r'^s390-ccw-virtio.*': 'sclpconsole', >>>>>>> } >>>>>>> >>>>>>> +#: Maps archictures to the preferred machine type >>>>>>> +MACHINE_TYPES = { >>>>>>> + r'^aarch64$': 'virt', >>>>>>> + r'^ppc$': 'g3beige', >>>>>>> + r'^ppc64$': 'pseries', >>>>>>> + r'^s390x$': 's390-ccw-virtio', >>>>>>> + r'^x86_64$': 'q35', >>>>>> >>>>>> Why choose Q35 rather than PC (the default)? >>>>>> >>>>>> I was wondering about how to generate variants/machines.json but this is >>>>>> definitively something we want to do via a QMP query. >>>>>> >>>>>> Eduardo what do you think? >>>>>> >>>>> >>>>> It was motivated by Eduardo's initiative to make q35 the default "across >>>>> the board". He can confirm and give more details. >>>> >>>> Making Q35 the default on applications using QEMU and libvirt is >>>> something I'd like to happen. But I think the simplest way to do >>>> that is to change the QEMU default. This way you won't need this >>>> table on qemu.py: you can just use the default provided by QEMU. >>>> >>> >>> The idea is to bring consistency on how we're calling >>> "qemu-system-$(ARCH)", and at the same time apply the "explicit is >>> better than implicit" rule. >>> >>> The most important fact is that some targets do not (currently) have >>> "the default provided by QEMU", aarch64 is one of them. >>> >>> - Cleber. >>> >> >> So I ended up not relaying the question properly: should we default >> (even if explicitly adding "-machine") to "pc"? > > I think using the default machine-type (when QEMU has a default) > would be less surprising for users of the qemu.py API. > > Implicitly adding -machine when there's no default is also > surprising, but then it's a nice surprise: instead of crashing > you get a running VM. > > Now, there are two other questions related to this: > > If using 'pc' as default, should we always add -machine, or just > omit the machine-type name? I think we should omit it unless the > caller asked for a specific machine-type name (because it would > be less surprising for users of the API). I agree with that. > > About our default testing configuration for acceptance tests: > should acceptance tests run against PC by default? Should it > test Q35? Should we test both PC and Q35? I'm not sure what's > the answer, but I think these decisions shouldn't affect the > qemu.py API at all. If I'm going to submit contributions to some subsystem, I'd like to run all the tests that cover this subsystem, previous to annoy the maintainer. For example if a series target the "X86 Machines" subsystem, then I'd expect the JSON variant to test both PC and Q35. Regards, Phil.
On Wed, Oct 10, 2018 at 11:28:40AM -0300, Eduardo Habkost wrote: > On Wed, Oct 10, 2018 at 10:15:15AM -0400, Cleber Rosa wrote: > > > > > > On 10/10/18 9:59 AM, Cleber Rosa wrote: > > > > > > > > > On 10/10/18 9:46 AM, Eduardo Habkost wrote: > > >> On Wed, Oct 10, 2018 at 08:35:38AM -0400, Cleber Rosa wrote: > > >>> > > >>> > > >>> On 10/10/18 7:00 AM, Philippe Mathieu-Daudé wrote: > > >>>> On 10/10/2018 01:26, Cleber Rosa wrote: > > >>>>> Some targets require a machine type to be set, as there's no default > > >>>>> (aarch64 is one example). To give a consistent interface to users of > > >>>>> this API, this changes set_machine() so that a predefined default can > > >>>>> be used, if one is not given. The approach used is exactly the same > > >>>>> with the console device type. > > >>>>> > > >>>>> Also, even when there's a default machine type, for some purposes, > > >>>>> testing included, it's better if outside code is explicit about the > > >>>>> machine type, instead of relying on whatever is set internally. > > >>>>> > > >>>>> Signed-off-by: Cleber Rosa <crosa@redhat.com> > > >>>>> --- > > >>>>> scripts/qemu.py | 22 +++++++++++++++++++++- > > >>>>> 1 file changed, 21 insertions(+), 1 deletion(-) > > >>>>> > > >>>>> diff --git a/scripts/qemu.py b/scripts/qemu.py > > >>>>> index d9e24a0c1a..fca9b76990 100644 > > >>>>> --- a/scripts/qemu.py > > >>>>> +++ b/scripts/qemu.py > > >>>>> @@ -36,6 +36,15 @@ CONSOLE_DEV_TYPES = { > > >>>>> r'^s390-ccw-virtio.*': 'sclpconsole', > > >>>>> } > > >>>>> > > >>>>> +#: Maps archictures to the preferred machine type > > >>>>> +MACHINE_TYPES = { > > >>>>> + r'^aarch64$': 'virt', > > >>>>> + r'^ppc$': 'g3beige', > > >>>>> + r'^ppc64$': 'pseries', > > >>>>> + r'^s390x$': 's390-ccw-virtio', > > >>>>> + r'^x86_64$': 'q35', > > >>>> > > >>>> Why choose Q35 rather than PC (the default)? > > >>>> > > >>>> I was wondering about how to generate variants/machines.json but this is > > >>>> definitively something we want to do via a QMP query. > > >>>> > > >>>> Eduardo what do you think? > > >>>> > > >>> > > >>> It was motivated by Eduardo's initiative to make q35 the default "across > > >>> the board". He can confirm and give more details. > > >> > > >> Making Q35 the default on applications using QEMU and libvirt is > > >> something I'd like to happen. But I think the simplest way to do > > >> that is to change the QEMU default. This way you won't need this > > >> table on qemu.py: you can just use the default provided by QEMU. > > >> > > > > > > The idea is to bring consistency on how we're calling > > > "qemu-system-$(ARCH)", and at the same time apply the "explicit is > > > better than implicit" rule. > > > > > > The most important fact is that some targets do not (currently) have > > > "the default provided by QEMU", aarch64 is one of them. > > > > > > - Cleber. > > > > > > > So I ended up not relaying the question properly: should we default > > (even if explicitly adding "-machine") to "pc"? > > I think using the default machine-type (when QEMU has a default) > would be less surprising for users of the qemu.py API. > > Implicitly adding -machine when there's no default is also > surprising, but then it's a nice surprise: instead of crashing > you get a running VM. > > Now, there are two other questions related to this: > > If using 'pc' as default, should we always add -machine, or just > omit the machine-type name? I think we should omit it unless the > caller asked for a specific machine-type name (because it would > be less surprising for users of the API). > > About our default testing configuration for acceptance tests: > should acceptance tests run against PC by default? Should it > test Q35? Should we test both PC and Q35? I'm not sure what's > the answer, but I think these decisions shouldn't affect the > qemu.py API at all. As a general goal we should be aiming to test everything we provide to users if, we expect them to actually use it and not have it break later :-) Given that 'pc' has been the default for entire existance of QEMU, 99% of guests are using 'pc' and thus we definitely want to be testing it. Equally we'd like to encourage use of 'q35' so we should also be testing that, even though it has very little usage right now. Thus, we really need to be running any tests against both machine types. If we do that, then the question of default doesn't really matter as one of them is always not the default and needs testing regardless. Regards, Daniel
On 10/10/18 10:28 AM, Eduardo Habkost wrote: > On Wed, Oct 10, 2018 at 10:15:15AM -0400, Cleber Rosa wrote: >> >> >> On 10/10/18 9:59 AM, Cleber Rosa wrote: >>> >>> >>> On 10/10/18 9:46 AM, Eduardo Habkost wrote: >>>> On Wed, Oct 10, 2018 at 08:35:38AM -0400, Cleber Rosa wrote: >>>>> >>>>> >>>>> On 10/10/18 7:00 AM, Philippe Mathieu-Daudé wrote: >>>>>> On 10/10/2018 01:26, Cleber Rosa wrote: >>>>>>> Some targets require a machine type to be set, as there's no default >>>>>>> (aarch64 is one example). To give a consistent interface to users of >>>>>>> this API, this changes set_machine() so that a predefined default can >>>>>>> be used, if one is not given. The approach used is exactly the same >>>>>>> with the console device type. >>>>>>> >>>>>>> Also, even when there's a default machine type, for some purposes, >>>>>>> testing included, it's better if outside code is explicit about the >>>>>>> machine type, instead of relying on whatever is set internally. >>>>>>> >>>>>>> Signed-off-by: Cleber Rosa <crosa@redhat.com> >>>>>>> --- >>>>>>> scripts/qemu.py | 22 +++++++++++++++++++++- >>>>>>> 1 file changed, 21 insertions(+), 1 deletion(-) >>>>>>> >>>>>>> diff --git a/scripts/qemu.py b/scripts/qemu.py >>>>>>> index d9e24a0c1a..fca9b76990 100644 >>>>>>> --- a/scripts/qemu.py >>>>>>> +++ b/scripts/qemu.py >>>>>>> @@ -36,6 +36,15 @@ CONSOLE_DEV_TYPES = { >>>>>>> r'^s390-ccw-virtio.*': 'sclpconsole', >>>>>>> } >>>>>>> >>>>>>> +#: Maps archictures to the preferred machine type >>>>>>> +MACHINE_TYPES = { >>>>>>> + r'^aarch64$': 'virt', >>>>>>> + r'^ppc$': 'g3beige', >>>>>>> + r'^ppc64$': 'pseries', >>>>>>> + r'^s390x$': 's390-ccw-virtio', >>>>>>> + r'^x86_64$': 'q35', >>>>>> >>>>>> Why choose Q35 rather than PC (the default)? >>>>>> >>>>>> I was wondering about how to generate variants/machines.json but this is >>>>>> definitively something we want to do via a QMP query. >>>>>> >>>>>> Eduardo what do you think? >>>>>> >>>>> >>>>> It was motivated by Eduardo's initiative to make q35 the default "across >>>>> the board". He can confirm and give more details. >>>> >>>> Making Q35 the default on applications using QEMU and libvirt is >>>> something I'd like to happen. But I think the simplest way to do >>>> that is to change the QEMU default. This way you won't need this >>>> table on qemu.py: you can just use the default provided by QEMU. >>>> >>> >>> The idea is to bring consistency on how we're calling >>> "qemu-system-$(ARCH)", and at the same time apply the "explicit is >>> better than implicit" rule. >>> >>> The most important fact is that some targets do not (currently) have >>> "the default provided by QEMU", aarch64 is one of them. >>> >>> - Cleber. >>> >> >> So I ended up not relaying the question properly: should we default >> (even if explicitly adding "-machine") to "pc"? > > I think using the default machine-type (when QEMU has a default) > would be less surprising for users of the qemu.py API. > OK, agreed. > Implicitly adding -machine when there's no default is also > surprising, but then it's a nice surprise: instead of crashing > you get a running VM. > > Now, there are two other questions related to this: > > If using 'pc' as default, should we always add -machine, or just > omit the machine-type name? I think we should omit it unless the > caller asked for a specific machine-type name (because it would > be less surprising for users of the API). > OK. > About our default testing configuration for acceptance tests: > should acceptance tests run against PC by default? Should it > test Q35? Should we test both PC and Q35? I'm not sure what's > the answer, but I think these decisions shouldn't affect the > qemu.py API at all. > OK. To make sure we're on the same page, we're still going to have default machine types, based on the arch, for those targets that don't provide one (aarch64 is one example). Right? - Cleber.
On 10/10/18 11:26 AM, Philippe Mathieu-Daudé wrote: > On 10/10/2018 16:28, Eduardo Habkost wrote: >> On Wed, Oct 10, 2018 at 10:15:15AM -0400, Cleber Rosa wrote: >>> >>> >>> On 10/10/18 9:59 AM, Cleber Rosa wrote: >>>> >>>> >>>> On 10/10/18 9:46 AM, Eduardo Habkost wrote: >>>>> On Wed, Oct 10, 2018 at 08:35:38AM -0400, Cleber Rosa wrote: >>>>>> >>>>>> >>>>>> On 10/10/18 7:00 AM, Philippe Mathieu-Daudé wrote: >>>>>>> On 10/10/2018 01:26, Cleber Rosa wrote: >>>>>>>> Some targets require a machine type to be set, as there's no default >>>>>>>> (aarch64 is one example). To give a consistent interface to users of >>>>>>>> this API, this changes set_machine() so that a predefined default can >>>>>>>> be used, if one is not given. The approach used is exactly the same >>>>>>>> with the console device type. >>>>>>>> >>>>>>>> Also, even when there's a default machine type, for some purposes, >>>>>>>> testing included, it's better if outside code is explicit about the >>>>>>>> machine type, instead of relying on whatever is set internally. >>>>>>>> >>>>>>>> Signed-off-by: Cleber Rosa <crosa@redhat.com> >>>>>>>> --- >>>>>>>> scripts/qemu.py | 22 +++++++++++++++++++++- >>>>>>>> 1 file changed, 21 insertions(+), 1 deletion(-) >>>>>>>> >>>>>>>> diff --git a/scripts/qemu.py b/scripts/qemu.py >>>>>>>> index d9e24a0c1a..fca9b76990 100644 >>>>>>>> --- a/scripts/qemu.py >>>>>>>> +++ b/scripts/qemu.py >>>>>>>> @@ -36,6 +36,15 @@ CONSOLE_DEV_TYPES = { >>>>>>>> r'^s390-ccw-virtio.*': 'sclpconsole', >>>>>>>> } >>>>>>>> >>>>>>>> +#: Maps archictures to the preferred machine type >>>>>>>> +MACHINE_TYPES = { >>>>>>>> + r'^aarch64$': 'virt', >>>>>>>> + r'^ppc$': 'g3beige', >>>>>>>> + r'^ppc64$': 'pseries', >>>>>>>> + r'^s390x$': 's390-ccw-virtio', >>>>>>>> + r'^x86_64$': 'q35', >>>>>>> >>>>>>> Why choose Q35 rather than PC (the default)? >>>>>>> >>>>>>> I was wondering about how to generate variants/machines.json but this is >>>>>>> definitively something we want to do via a QMP query. >>>>>>> >>>>>>> Eduardo what do you think? >>>>>>> >>>>>> >>>>>> It was motivated by Eduardo's initiative to make q35 the default "across >>>>>> the board". He can confirm and give more details. >>>>> >>>>> Making Q35 the default on applications using QEMU and libvirt is >>>>> something I'd like to happen. But I think the simplest way to do >>>>> that is to change the QEMU default. This way you won't need this >>>>> table on qemu.py: you can just use the default provided by QEMU. >>>>> >>>> >>>> The idea is to bring consistency on how we're calling >>>> "qemu-system-$(ARCH)", and at the same time apply the "explicit is >>>> better than implicit" rule. >>>> >>>> The most important fact is that some targets do not (currently) have >>>> "the default provided by QEMU", aarch64 is one of them. >>>> >>>> - Cleber. >>>> >>> >>> So I ended up not relaying the question properly: should we default >>> (even if explicitly adding "-machine") to "pc"? >> >> I think using the default machine-type (when QEMU has a default) >> would be less surprising for users of the qemu.py API. >> >> Implicitly adding -machine when there's no default is also >> surprising, but then it's a nice surprise: instead of crashing >> you get a running VM. >> >> Now, there are two other questions related to this: >> >> If using 'pc' as default, should we always add -machine, or just >> omit the machine-type name? I think we should omit it unless the >> caller asked for a specific machine-type name (because it would >> be less surprising for users of the API). > > I agree with that. > OK! >> >> About our default testing configuration for acceptance tests: >> should acceptance tests run against PC by default? Should it >> test Q35? Should we test both PC and Q35? I'm not sure what's >> the answer, but I think these decisions shouldn't affect the >> qemu.py API at all. > > If I'm going to submit contributions to some subsystem, I'd like to run > all the tests that cover this subsystem, previous to annoy the maintainer. > > For example if a series target the "X86 Machines" subsystem, then I'd > expect the JSON variant to test both PC and Q35. > I agree, and we'll get there, but I'd rather do it in small steps. The reason is that we want every single FAIL/ERROR on the acceptance tests to really flag a regression, so we need careful execution and validation prior to increasing the "test matrix". At the same time, we need to be careful to not grow the default acceptance tests execution to a point that people won't run it. I've just heard similar feedback regarding Avocado-VT, that has *too many* tests that make it impractical for the individual developer to run. The plans we have for that is to define some sane test sets (probably based on tags and/or variants files), and at the same time, rely on combinatorial independent testing to reduce the number of test variations to make it practical for the regular developer to run on his environment. Regards! - Cleber. > Regards, > > Phil. >
On 10/10/18 11:31 AM, Daniel P. Berrangé wrote: > On Wed, Oct 10, 2018 at 11:28:40AM -0300, Eduardo Habkost wrote: >> On Wed, Oct 10, 2018 at 10:15:15AM -0400, Cleber Rosa wrote: >>> >>> >>> On 10/10/18 9:59 AM, Cleber Rosa wrote: >>>> >>>> >>>> On 10/10/18 9:46 AM, Eduardo Habkost wrote: >>>>> On Wed, Oct 10, 2018 at 08:35:38AM -0400, Cleber Rosa wrote: >>>>>> >>>>>> >>>>>> On 10/10/18 7:00 AM, Philippe Mathieu-Daudé wrote: >>>>>>> On 10/10/2018 01:26, Cleber Rosa wrote: >>>>>>>> Some targets require a machine type to be set, as there's no default >>>>>>>> (aarch64 is one example). To give a consistent interface to users of >>>>>>>> this API, this changes set_machine() so that a predefined default can >>>>>>>> be used, if one is not given. The approach used is exactly the same >>>>>>>> with the console device type. >>>>>>>> >>>>>>>> Also, even when there's a default machine type, for some purposes, >>>>>>>> testing included, it's better if outside code is explicit about the >>>>>>>> machine type, instead of relying on whatever is set internally. >>>>>>>> >>>>>>>> Signed-off-by: Cleber Rosa <crosa@redhat.com> >>>>>>>> --- >>>>>>>> scripts/qemu.py | 22 +++++++++++++++++++++- >>>>>>>> 1 file changed, 21 insertions(+), 1 deletion(-) >>>>>>>> >>>>>>>> diff --git a/scripts/qemu.py b/scripts/qemu.py >>>>>>>> index d9e24a0c1a..fca9b76990 100644 >>>>>>>> --- a/scripts/qemu.py >>>>>>>> +++ b/scripts/qemu.py >>>>>>>> @@ -36,6 +36,15 @@ CONSOLE_DEV_TYPES = { >>>>>>>> r'^s390-ccw-virtio.*': 'sclpconsole', >>>>>>>> } >>>>>>>> >>>>>>>> +#: Maps archictures to the preferred machine type >>>>>>>> +MACHINE_TYPES = { >>>>>>>> + r'^aarch64$': 'virt', >>>>>>>> + r'^ppc$': 'g3beige', >>>>>>>> + r'^ppc64$': 'pseries', >>>>>>>> + r'^s390x$': 's390-ccw-virtio', >>>>>>>> + r'^x86_64$': 'q35', >>>>>>> >>>>>>> Why choose Q35 rather than PC (the default)? >>>>>>> >>>>>>> I was wondering about how to generate variants/machines.json but this is >>>>>>> definitively something we want to do via a QMP query. >>>>>>> >>>>>>> Eduardo what do you think? >>>>>>> >>>>>> >>>>>> It was motivated by Eduardo's initiative to make q35 the default "across >>>>>> the board". He can confirm and give more details. >>>>> >>>>> Making Q35 the default on applications using QEMU and libvirt is >>>>> something I'd like to happen. But I think the simplest way to do >>>>> that is to change the QEMU default. This way you won't need this >>>>> table on qemu.py: you can just use the default provided by QEMU. >>>>> >>>> >>>> The idea is to bring consistency on how we're calling >>>> "qemu-system-$(ARCH)", and at the same time apply the "explicit is >>>> better than implicit" rule. >>>> >>>> The most important fact is that some targets do not (currently) have >>>> "the default provided by QEMU", aarch64 is one of them. >>>> >>>> - Cleber. >>>> >>> >>> So I ended up not relaying the question properly: should we default >>> (even if explicitly adding "-machine") to "pc"? >> >> I think using the default machine-type (when QEMU has a default) >> would be less surprising for users of the qemu.py API. >> >> Implicitly adding -machine when there's no default is also >> surprising, but then it's a nice surprise: instead of crashing >> you get a running VM. >> >> Now, there are two other questions related to this: >> >> If using 'pc' as default, should we always add -machine, or just >> omit the machine-type name? I think we should omit it unless the >> caller asked for a specific machine-type name (because it would >> be less surprising for users of the API). >> >> About our default testing configuration for acceptance tests: >> should acceptance tests run against PC by default? Should it >> test Q35? Should we test both PC and Q35? I'm not sure what's >> the answer, but I think these decisions shouldn't affect the >> qemu.py API at all. > > As a general goal we should be aiming to test everything we provide > to users if, we expect them to actually use it and not have it > break later :-) > > Given that 'pc' has been the default for entire existance of QEMU, > 99% of guests are using 'pc' and thus we definitely want to be > testing it. > > Equally we'd like to encourage use of 'q35' so we should also be > testing that, even though it has very little usage right now. > > Thus, we really need to be running any tests against both machine > types. If we do that, then the question of default doesn't really > matter as one of them is always not the default and needs testing > regardless. > > Regards, > Daniel > Hi Daniel, I think we're in sync. Please look at my previous reply (and accept my apologies if this is not a better "netiquette" than replying the same thing again). Regards! - Cleber.
On 10/10/2018 17:58, Cleber Rosa wrote: > > > On 10/10/18 11:26 AM, Philippe Mathieu-Daudé wrote: >> On 10/10/2018 16:28, Eduardo Habkost wrote: >>> On Wed, Oct 10, 2018 at 10:15:15AM -0400, Cleber Rosa wrote: >>>> >>>> >>>> On 10/10/18 9:59 AM, Cleber Rosa wrote: >>>>> >>>>> >>>>> On 10/10/18 9:46 AM, Eduardo Habkost wrote: >>>>>> On Wed, Oct 10, 2018 at 08:35:38AM -0400, Cleber Rosa wrote: >>>>>>> >>>>>>> >>>>>>> On 10/10/18 7:00 AM, Philippe Mathieu-Daudé wrote: >>>>>>>> On 10/10/2018 01:26, Cleber Rosa wrote: >>>>>>>>> Some targets require a machine type to be set, as there's no default >>>>>>>>> (aarch64 is one example). To give a consistent interface to users of >>>>>>>>> this API, this changes set_machine() so that a predefined default can >>>>>>>>> be used, if one is not given. The approach used is exactly the same >>>>>>>>> with the console device type. >>>>>>>>> >>>>>>>>> Also, even when there's a default machine type, for some purposes, >>>>>>>>> testing included, it's better if outside code is explicit about the >>>>>>>>> machine type, instead of relying on whatever is set internally. >>>>>>>>> >>>>>>>>> Signed-off-by: Cleber Rosa <crosa@redhat.com> >>>>>>>>> --- >>>>>>>>> scripts/qemu.py | 22 +++++++++++++++++++++- >>>>>>>>> 1 file changed, 21 insertions(+), 1 deletion(-) >>>>>>>>> >>>>>>>>> diff --git a/scripts/qemu.py b/scripts/qemu.py >>>>>>>>> index d9e24a0c1a..fca9b76990 100644 >>>>>>>>> --- a/scripts/qemu.py >>>>>>>>> +++ b/scripts/qemu.py >>>>>>>>> @@ -36,6 +36,15 @@ CONSOLE_DEV_TYPES = { >>>>>>>>> r'^s390-ccw-virtio.*': 'sclpconsole', >>>>>>>>> } >>>>>>>>> >>>>>>>>> +#: Maps archictures to the preferred machine type >>>>>>>>> +MACHINE_TYPES = { >>>>>>>>> + r'^aarch64$': 'virt', >>>>>>>>> + r'^ppc$': 'g3beige', >>>>>>>>> + r'^ppc64$': 'pseries', >>>>>>>>> + r'^s390x$': 's390-ccw-virtio', >>>>>>>>> + r'^x86_64$': 'q35', >>>>>>>> >>>>>>>> Why choose Q35 rather than PC (the default)? >>>>>>>> >>>>>>>> I was wondering about how to generate variants/machines.json but this is >>>>>>>> definitively something we want to do via a QMP query. >>>>>>>> >>>>>>>> Eduardo what do you think? >>>>>>>> >>>>>>> >>>>>>> It was motivated by Eduardo's initiative to make q35 the default "across >>>>>>> the board". He can confirm and give more details. >>>>>> >>>>>> Making Q35 the default on applications using QEMU and libvirt is >>>>>> something I'd like to happen. But I think the simplest way to do >>>>>> that is to change the QEMU default. This way you won't need this >>>>>> table on qemu.py: you can just use the default provided by QEMU. >>>>>> >>>>> >>>>> The idea is to bring consistency on how we're calling >>>>> "qemu-system-$(ARCH)", and at the same time apply the "explicit is >>>>> better than implicit" rule. >>>>> >>>>> The most important fact is that some targets do not (currently) have >>>>> "the default provided by QEMU", aarch64 is one of them. >>>>> >>>>> - Cleber. >>>>> >>>> >>>> So I ended up not relaying the question properly: should we default >>>> (even if explicitly adding "-machine") to "pc"? >>> >>> I think using the default machine-type (when QEMU has a default) >>> would be less surprising for users of the qemu.py API. >>> >>> Implicitly adding -machine when there's no default is also >>> surprising, but then it's a nice surprise: instead of crashing >>> you get a running VM. >>> >>> Now, there are two other questions related to this: >>> >>> If using 'pc' as default, should we always add -machine, or just >>> omit the machine-type name? I think we should omit it unless the >>> caller asked for a specific machine-type name (because it would >>> be less surprising for users of the API). >> >> I agree with that. >> > > OK! > >>> >>> About our default testing configuration for acceptance tests: >>> should acceptance tests run against PC by default? Should it >>> test Q35? Should we test both PC and Q35? I'm not sure what's >>> the answer, but I think these decisions shouldn't affect the >>> qemu.py API at all. >> >> If I'm going to submit contributions to some subsystem, I'd like to run >> all the tests that cover this subsystem, previous to annoy the maintainer. >> >> For example if a series target the "X86 Machines" subsystem, then I'd >> expect the JSON variant to test both PC and Q35. >> > > I agree, and we'll get there, but I'd rather do it in small steps. Sure. > > The reason is that we want every single FAIL/ERROR on the acceptance > tests to really flag a regression, so we need careful execution and > validation prior to increasing the "test matrix". > > At the same time, we need to be careful to not grow the default > acceptance tests execution to a point that people won't run it. I've > just heard similar feedback regarding Avocado-VT, that has *too many* > tests that make it impractical for the individual developer to run. The problem here is not the number of tests but the set of tests selected. A test should have a description of what it is testing, the covered devices/modes/... From this description it should be easy to add filtering rules. From a developer point of view, I'll run the tests covering the areas I modified. A maintainer runs more extensive tests on his subsystems. Now if you plan a release, you are not an individual developer :) > > The plans we have for that is to define some sane test sets (probably > based on tags and/or variants files), and at the same time, rely on > combinatorial independent testing to reduce the number of test > variations to make it practical for the regular developer to run on his > environment. OK, we'll get there! :) (I don't want to reject people to contribute tests because "we already have too many".) Regards, Phil.
On 10 October 2018 at 16:47, Cleber Rosa <crosa@redhat.com> wrote: > To make sure we're on the same page, we're still going to have default > machine types, based on the arch, for those targets that don't provide > one (aarch64 is one example). Right? Does it make sense to define a default? The reason arm doesn't specify a default machine type is because you can't just run any old guest on any old machine type. You need to know "this guest image will run on machine type X", and run it on machine type X. This is like knowing you need to run a test on x86 PC and not on PPC spapr. Would it make more sense for each test to specify which machine types it can work on? thanks -- PMM
On 10/10/18 12:23 PM, Peter Maydell wrote: > On 10 October 2018 at 16:47, Cleber Rosa <crosa@redhat.com> wrote: >> To make sure we're on the same page, we're still going to have default >> machine types, based on the arch, for those targets that don't provide >> one (aarch64 is one example). Right? > > Does it make sense to define a default? The reason arm > doesn't specify a default machine type is because you > can't just run any old guest on any old machine type. > You need to know "this guest image will run on machine > type X", and run it on machine type X. This is like > knowing you need to run a test on x86 PC and not > on PPC spapr. > While requiring tests to specify every single aspect of the environment that will be used may be OK for low level unit tests, it puts a lot of burden on higher level tests (which is supposed to be the vast majority under tests/acceptance). From a test writer perspective, working on these higher level tests, it may want to make sure that feature "X", unrelated to the target arch, machine type, etc, "just works". You man want to look at the "vnc.py" test for a real world example. Eduardo has suggested that "make check-acceptance" runs all (possible) tests on all target archs by default. > Would it make more sense for each test to specify > which machine types it can work on? > I think it does, but I believe in the black list approach, instead of the white list. The reason for that is that I believe that majority of the tests under "tests/acceptance" can be made to work on every target (which would be the default). So far, I've made sure tests behave correctly on the 5 arches included in the "archs.json" file in this series (x86_64, ppc64, ppc, aarch64, s390x). To give a full disclosure, "boot_linux.py" (boots a linux kernel) is x86_64 specific, and CANCELS when asked to be run on other archs. But, on the work I've done top of these series, it already works with ppc64 and aarch64. Also, "boot_linux.py" sent in another series, (which boots a full linux guest) is also being adapted to work on most of the target archs. And, as an exception, you can still define/check the arch and machine type *in the test*, if it's not supposed to work on most. Does that make sense? - Cleber. > thanks > -- PMM >
On 10 October 2018 at 18:52, Cleber Rosa <crosa@redhat.com> wrote: > > > On 10/10/18 12:23 PM, Peter Maydell wrote: >> On 10 October 2018 at 16:47, Cleber Rosa <crosa@redhat.com> wrote: >>> To make sure we're on the same page, we're still going to have default >>> machine types, based on the arch, for those targets that don't provide >>> one (aarch64 is one example). Right? >> >> Does it make sense to define a default? The reason arm >> doesn't specify a default machine type is because you >> can't just run any old guest on any old machine type. >> You need to know "this guest image will run on machine >> type X", and run it on machine type X. This is like >> knowing you need to run a test on x86 PC and not >> on PPC spapr. >> > > While requiring tests to specify every single aspect of the environment > that will be used may be OK for low level unit tests, it puts a lot of > burden on higher level tests (which is supposed to be the vast majority > under tests/acceptance). > > From a test writer perspective, working on these higher level tests, it > may want to make sure that feature "X", unrelated to the target arch, > machine type, etc, "just works". You man want to look at the "vnc.py" > test for a real world example. OK, if it doesn't have a dependency on machine at all, it should state that somehow. > Eduardo has suggested that "make check-acceptance" runs all (possible) > tests on all target archs by default. Yeah; or we have some mechanism for trimming down the matrix of what we run. But I think it's better coverage if we have 3 tests ABC that don't depend on machine and 3 machines XYZ to run AX BY CZ than AX BX CX by specifying X as an arbitrary "default". It looks like the 'vnc' test is just testing QEMU functionality, not anything that involves interacting with the guest or machine model? There's a good argument that that only really needs to be run once, not once per architecture. You might also want to consider the "none" machine, which exists for bits of test infrastructure that aren't actually trying to run guests. >> Would it make more sense for each test to specify >> which machine types it can work on? >> > > I think it does, but I believe in the black list approach, instead of > the white list. > > The reason for that is that I believe that majority of the tests under > "tests/acceptance" can be made to work on every target (which would be > the default). So far, I've made sure tests behave correctly on the 5 > arches included in the "archs.json" file in this series (x86_64, ppc64, > ppc, aarch64, s390x). > > To give a full disclosure, "boot_linux.py" (boots a linux kernel) is > x86_64 specific, and CANCELS when asked to be run on other archs. But, > on the work I've done top of these series, it already works with ppc64 > and aarch64. Also, "boot_linux.py" sent in another series, (which boots > a full linux guest) is also being adapted to work on most of the target > archs. Right, "boot Linux" is machine specific. The kernel/disk /etc that boots on aarch64 virt is probably not going to boot on the 64-bit xilinx board; and on 32-bit arm you definitely are going to want a different kernel in some places. This is likely to be true of most tests that actually try to run code in the guest. We should aim to test the machines we care about (regardless of what architectures they are), rather than thinking about it in terms of "testing architectures X, Y, Z", I think. I think you're going to need at least some whitelist functionality; otherwise half the tests are going to break every time we add a new machine (and "add every new machine to the blacklist for half the tests" doesn't scale very well). thanks -- PMM
On 10/10/18 12:08 PM, Philippe Mathieu-Daudé wrote: > On 10/10/2018 17:58, Cleber Rosa wrote: >> >> >> On 10/10/18 11:26 AM, Philippe Mathieu-Daudé wrote: >>> On 10/10/2018 16:28, Eduardo Habkost wrote: >>>> On Wed, Oct 10, 2018 at 10:15:15AM -0400, Cleber Rosa wrote: >>>>> >>>>> >>>>> On 10/10/18 9:59 AM, Cleber Rosa wrote: >>>>>> >>>>>> >>>>>> On 10/10/18 9:46 AM, Eduardo Habkost wrote: >>>>>>> On Wed, Oct 10, 2018 at 08:35:38AM -0400, Cleber Rosa wrote: >>>>>>>> >>>>>>>> >>>>>>>> On 10/10/18 7:00 AM, Philippe Mathieu-Daudé wrote: >>>>>>>>> On 10/10/2018 01:26, Cleber Rosa wrote: >>>>>>>>>> Some targets require a machine type to be set, as there's no default >>>>>>>>>> (aarch64 is one example). To give a consistent interface to users of >>>>>>>>>> this API, this changes set_machine() so that a predefined default can >>>>>>>>>> be used, if one is not given. The approach used is exactly the same >>>>>>>>>> with the console device type. >>>>>>>>>> >>>>>>>>>> Also, even when there's a default machine type, for some purposes, >>>>>>>>>> testing included, it's better if outside code is explicit about the >>>>>>>>>> machine type, instead of relying on whatever is set internally. >>>>>>>>>> >>>>>>>>>> Signed-off-by: Cleber Rosa <crosa@redhat.com> >>>>>>>>>> --- >>>>>>>>>> scripts/qemu.py | 22 +++++++++++++++++++++- >>>>>>>>>> 1 file changed, 21 insertions(+), 1 deletion(-) >>>>>>>>>> >>>>>>>>>> diff --git a/scripts/qemu.py b/scripts/qemu.py >>>>>>>>>> index d9e24a0c1a..fca9b76990 100644 >>>>>>>>>> --- a/scripts/qemu.py >>>>>>>>>> +++ b/scripts/qemu.py >>>>>>>>>> @@ -36,6 +36,15 @@ CONSOLE_DEV_TYPES = { >>>>>>>>>> r'^s390-ccw-virtio.*': 'sclpconsole', >>>>>>>>>> } >>>>>>>>>> >>>>>>>>>> +#: Maps archictures to the preferred machine type >>>>>>>>>> +MACHINE_TYPES = { >>>>>>>>>> + r'^aarch64$': 'virt', >>>>>>>>>> + r'^ppc$': 'g3beige', >>>>>>>>>> + r'^ppc64$': 'pseries', >>>>>>>>>> + r'^s390x$': 's390-ccw-virtio', >>>>>>>>>> + r'^x86_64$': 'q35', >>>>>>>>> >>>>>>>>> Why choose Q35 rather than PC (the default)? >>>>>>>>> >>>>>>>>> I was wondering about how to generate variants/machines.json but this is >>>>>>>>> definitively something we want to do via a QMP query. >>>>>>>>> >>>>>>>>> Eduardo what do you think? >>>>>>>>> >>>>>>>> >>>>>>>> It was motivated by Eduardo's initiative to make q35 the default "across >>>>>>>> the board". He can confirm and give more details. >>>>>>> >>>>>>> Making Q35 the default on applications using QEMU and libvirt is >>>>>>> something I'd like to happen. But I think the simplest way to do >>>>>>> that is to change the QEMU default. This way you won't need this >>>>>>> table on qemu.py: you can just use the default provided by QEMU. >>>>>>> >>>>>> >>>>>> The idea is to bring consistency on how we're calling >>>>>> "qemu-system-$(ARCH)", and at the same time apply the "explicit is >>>>>> better than implicit" rule. >>>>>> >>>>>> The most important fact is that some targets do not (currently) have >>>>>> "the default provided by QEMU", aarch64 is one of them. >>>>>> >>>>>> - Cleber. >>>>>> >>>>> >>>>> So I ended up not relaying the question properly: should we default >>>>> (even if explicitly adding "-machine") to "pc"? >>>> >>>> I think using the default machine-type (when QEMU has a default) >>>> would be less surprising for users of the qemu.py API. >>>> >>>> Implicitly adding -machine when there's no default is also >>>> surprising, but then it's a nice surprise: instead of crashing >>>> you get a running VM. >>>> >>>> Now, there are two other questions related to this: >>>> >>>> If using 'pc' as default, should we always add -machine, or just >>>> omit the machine-type name? I think we should omit it unless the >>>> caller asked for a specific machine-type name (because it would >>>> be less surprising for users of the API). >>> >>> I agree with that. >>> >> >> OK! >> >>>> >>>> About our default testing configuration for acceptance tests: >>>> should acceptance tests run against PC by default? Should it >>>> test Q35? Should we test both PC and Q35? I'm not sure what's >>>> the answer, but I think these decisions shouldn't affect the >>>> qemu.py API at all. >>> >>> If I'm going to submit contributions to some subsystem, I'd like to run >>> all the tests that cover this subsystem, previous to annoy the maintainer. >>> >>> For example if a series target the "X86 Machines" subsystem, then I'd >>> expect the JSON variant to test both PC and Q35. >>> >> >> I agree, and we'll get there, but I'd rather do it in small steps. > > Sure. > >> >> The reason is that we want every single FAIL/ERROR on the acceptance >> tests to really flag a regression, so we need careful execution and >> validation prior to increasing the "test matrix". >> >> At the same time, we need to be careful to not grow the default >> acceptance tests execution to a point that people won't run it. I've >> just heard similar feedback regarding Avocado-VT, that has *too many* >> tests that make it impractical for the individual developer to run. > > The problem here is not the number of tests but the set of tests selected. > > A test should have a description of what it is testing, the covered > devices/modes/... > From this description it should be easy to add filtering rules. > > From a developer point of view, I'll run the tests covering the areas I > modified. > A maintainer runs more extensive tests on his subsystems. > And a CI system may run even more. And QE will hopefully run an absurd number of tests. And, all of those groups will be working and collaborating on the same code base, with no secret sauces. "I have a dream", and that dream is developer A reporting a test failure, and developer B checking the CI/QE database to find out that it also happens on different scenarios - or only on a specific scenario. IMO this can quickly point you towards the failure cause. > Now if you plan a release, you are not an individual developer :) > Yep, true. >> >> The plans we have for that is to define some sane test sets (probably >> based on tags and/or variants files), and at the same time, rely on >> combinatorial independent testing to reduce the number of test >> variations to make it practical for the regular developer to run on his >> environment. > > OK, we'll get there! :) > > (I don't want to reject people to contribute tests because "we already > have too many".) > Oh no, I agree with you. I'm already envisioning a few things: * Proposing a set of tags * Combinatorial Independent Testing * Maybe breaking down the "tests/acceptance" directory into target and non-target specific (think of "qemu-img" only tests) * Using the Avocado Job API to define the test suites (some old example, still Avocado-VT related: https://www.redhat.com/archives/avocado-devel/2018-April/msg00015.html) Regards, - Cleber. > Regards, > > Phil. >
On 10/10/18 2:07 PM, Peter Maydell wrote: > On 10 October 2018 at 18:52, Cleber Rosa <crosa@redhat.com> wrote: >> >> >> On 10/10/18 12:23 PM, Peter Maydell wrote: >>> On 10 October 2018 at 16:47, Cleber Rosa <crosa@redhat.com> wrote: >>>> To make sure we're on the same page, we're still going to have default >>>> machine types, based on the arch, for those targets that don't provide >>>> one (aarch64 is one example). Right? >>> >>> Does it make sense to define a default? The reason arm >>> doesn't specify a default machine type is because you >>> can't just run any old guest on any old machine type. >>> You need to know "this guest image will run on machine >>> type X", and run it on machine type X. This is like >>> knowing you need to run a test on x86 PC and not >>> on PPC spapr. >>> >> >> While requiring tests to specify every single aspect of the environment >> that will be used may be OK for low level unit tests, it puts a lot of >> burden on higher level tests (which is supposed to be the vast majority >> under tests/acceptance). >> >> From a test writer perspective, working on these higher level tests, it >> may want to make sure that feature "X", unrelated to the target arch, >> machine type, etc, "just works". You man want to look at the "vnc.py" >> test for a real world example. > > OK, if it doesn't have a dependency on machine at all, it > should state that somehow. > >> Eduardo has suggested that "make check-acceptance" runs all (possible) >> tests on all target archs by default. > > Yeah; or we have some mechanism for trimming down the > matrix of what we run. But I think it's better coverage > if we have 3 tests ABC that don't depend on machine > and 3 machines XYZ to run AX BY CZ than AX BX CX by > specifying X as an arbitrary "default". > > It looks like the 'vnc' test is just testing QEMU functionality, > not anything that involves interacting with the guest or > machine model? There's a good argument that that only really > needs to be run once, not once per architecture. > > You might also want to consider the "none" machine, which exists > for bits of test infrastructure that aren't actually trying to > run guests. > >>> Would it make more sense for each test to specify >>> which machine types it can work on? >>> >> >> I think it does, but I believe in the black list approach, instead of >> the white list. >> >> The reason for that is that I believe that majority of the tests under >> "tests/acceptance" can be made to work on every target (which would be >> the default). So far, I've made sure tests behave correctly on the 5 >> arches included in the "archs.json" file in this series (x86_64, ppc64, >> ppc, aarch64, s390x). >> >> To give a full disclosure, "boot_linux.py" (boots a linux kernel) is >> x86_64 specific, and CANCELS when asked to be run on other archs. But, >> on the work I've done top of these series, it already works with ppc64 >> and aarch64. Also, "boot_linux.py" sent in another series, (which boots >> a full linux guest) is also being adapted to work on most of the target >> archs. > > Right, "boot Linux" is machine specific. The kernel/disk > /etc that boots on aarch64 virt is probably not going to boot > on the 64-bit xilinx board; and on 32-bit arm you definitely > are going to want a different kernel in some places. This > is likely to be true of most tests that actually try to run > code in the guest. > Agreed. > We should aim to test the machines we care about (regardless > of what architectures they are), rather than thinking about it > in terms of "testing architectures X, Y, Z", I think. > To me it's clear that: 1) I lack a complete understanding of what "we care about" 2) It's easier to start with something, and tweak it to taste And TBH, I fully agree with Philippe in the sense that difference developer/maintainer roles will require a different test "profile". > I think you're going to need at least some whitelist functionality; > otherwise half the tests are going to break every time we add > a new machine (and "add every new machine to the blacklist for > half the tests" doesn't scale very well). > The whitelist approach is in effect, it's the reason I sent a "archs.json" file, not with machine types, but with the archs that I've tested with. So, I'm going to push forward this series (a v3) with the same simplified approach, that is, `make check-acceptance` will still run the tests on a single target. Then, once that is settled, we can decide on: 1) `make check-acceptance` becomes "run on all target args/machine types" and `make check-acceptance-$(ARCH)-$(MACHINE)` is introduced. 2) `make check-acceptance-all` is introduced. To me it's clear that there's a huge continuation to this discussion, and that we should bite one piece at a time. Thoughts? Regards! - Cleber. > thanks > -- PMM >
On 10/10/18 11:47 AM, Cleber Rosa wrote: > > > On 10/10/18 10:28 AM, Eduardo Habkost wrote: >> On Wed, Oct 10, 2018 at 10:15:15AM -0400, Cleber Rosa wrote: >>> >>> >>> On 10/10/18 9:59 AM, Cleber Rosa wrote: >>>> >>>> >>>> On 10/10/18 9:46 AM, Eduardo Habkost wrote: >>>>> On Wed, Oct 10, 2018 at 08:35:38AM -0400, Cleber Rosa wrote: >>>>>> >>>>>> >>>>>> On 10/10/18 7:00 AM, Philippe Mathieu-Daudé wrote: >>>>>>> On 10/10/2018 01:26, Cleber Rosa wrote: >>>>>>>> Some targets require a machine type to be set, as there's no default >>>>>>>> (aarch64 is one example). To give a consistent interface to users of >>>>>>>> this API, this changes set_machine() so that a predefined default can >>>>>>>> be used, if one is not given. The approach used is exactly the same >>>>>>>> with the console device type. >>>>>>>> >>>>>>>> Also, even when there's a default machine type, for some purposes, >>>>>>>> testing included, it's better if outside code is explicit about the >>>>>>>> machine type, instead of relying on whatever is set internally. >>>>>>>> >>>>>>>> Signed-off-by: Cleber Rosa <crosa@redhat.com> >>>>>>>> --- >>>>>>>> scripts/qemu.py | 22 +++++++++++++++++++++- >>>>>>>> 1 file changed, 21 insertions(+), 1 deletion(-) >>>>>>>> >>>>>>>> diff --git a/scripts/qemu.py b/scripts/qemu.py >>>>>>>> index d9e24a0c1a..fca9b76990 100644 >>>>>>>> --- a/scripts/qemu.py >>>>>>>> +++ b/scripts/qemu.py >>>>>>>> @@ -36,6 +36,15 @@ CONSOLE_DEV_TYPES = { >>>>>>>> r'^s390-ccw-virtio.*': 'sclpconsole', >>>>>>>> } >>>>>>>> >>>>>>>> +#: Maps archictures to the preferred machine type >>>>>>>> +MACHINE_TYPES = { >>>>>>>> + r'^aarch64$': 'virt', >>>>>>>> + r'^ppc$': 'g3beige', >>>>>>>> + r'^ppc64$': 'pseries', >>>>>>>> + r'^s390x$': 's390-ccw-virtio', >>>>>>>> + r'^x86_64$': 'q35', >>>>>>> >>>>>>> Why choose Q35 rather than PC (the default)? >>>>>>> >>>>>>> I was wondering about how to generate variants/machines.json but this is >>>>>>> definitively something we want to do via a QMP query. >>>>>>> >>>>>>> Eduardo what do you think? >>>>>>> >>>>>> >>>>>> It was motivated by Eduardo's initiative to make q35 the default "across >>>>>> the board". He can confirm and give more details. >>>>> >>>>> Making Q35 the default on applications using QEMU and libvirt is >>>>> something I'd like to happen. But I think the simplest way to do >>>>> that is to change the QEMU default. This way you won't need this >>>>> table on qemu.py: you can just use the default provided by QEMU. >>>>> >>>> >>>> The idea is to bring consistency on how we're calling >>>> "qemu-system-$(ARCH)", and at the same time apply the "explicit is >>>> better than implicit" rule. >>>> >>>> The most important fact is that some targets do not (currently) have >>>> "the default provided by QEMU", aarch64 is one of them. >>>> >>>> - Cleber. >>>> >>> >>> So I ended up not relaying the question properly: should we default >>> (even if explicitly adding "-machine") to "pc"? >> >> I think using the default machine-type (when QEMU has a default) >> would be less surprising for users of the qemu.py API. >> > > OK, agreed. > >> Implicitly adding -machine when there's no default is also >> surprising, but then it's a nice surprise: instead of crashing >> you get a running VM. >> >> Now, there are two other questions related to this: >> >> If using 'pc' as default, should we always add -machine, or just >> omit the machine-type name? I think we should omit it unless the >> caller asked for a specific machine-type name (because it would >> be less surprising for users of the API). >> > Getting down to business, trying to apply those changes, I was faced with a situation. Actually, the same situation I faced a few months ago. Handling it was defered until it was *really* a blocker. Basically the issue is: the set_console() method, which gives tests a ready to use console, depends on knowing the machine type (see CONSOLE_DEV_TYPES). As a case study, let's look at "boot_console_linux.py": 1) it sets the machine type explicitly 2) it has nothing to do with the specific machine type 3) the setting of a machine type is boiler plate code to set a console 4) the console is used on the test's real purpose: verifying the Linux kernel booted Now, to be able to run the same test -- booting a Linux kernel -- on *other target archs*, we need the same machinery. Even more important: to have similar tests we'll need to either abstract those features or duplicate them. This can be seen, at least in part, on the firmware tests that Philippe sent to the list: they would also benefit from having a console device ready to be used on the configured machine type[1]: Assuming that we want to provide this type of machinery for free (or as close as that) to the acceptance/functional tests, we need some source of "known good" configuration for the targets we aim to support. Let's restrict the discussion to the issue at hand, machine types, while keeping in mind that the same pattern happened with devices types to use as console before, and my experience running into default network device types in further work (tests that interact with the guest by ssh'ing into it). The solutions I can think of are: 1) run the target binary previous to the "real" run, and query information -- this is what Avocado-VT does[2], and something I tried on earlier versions of the acceptance tests infrastructure code 2) attempt to get this information from the build system[3] 3) hard code the "known" good configuration I've previously worked on solutions along the lines of #1 and #2, but the general feedback wasn't that positive, for valid reasons. Eduardo probably remembers this. So, I'm tempted to try solution #3. As much as duplicating target defaults in qemu.py doesn't sound perfect, it seems to be the more predictable and attainable solution at this point. Thoughts? Thanks! - Cleber. [1] - https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg00591.html [2] - https://github.com/avocado-framework/avocado-vt/blob/65.0/virttest/utils_misc.py#L2105 [3] - http://lists.gnu.org/archive/html/qemu-devel/2017-07/msg06757.html > OK. > >> About our default testing configuration for acceptance tests: >> should acceptance tests run against PC by default? Should it >> test Q35? Should we test both PC and Q35? I'm not sure what's >> the answer, but I think these decisions shouldn't affect the >> qemu.py API at all. >> > > OK. > > To make sure we're on the same page, we're still going to have default > machine types, based on the arch, for those targets that don't provide > one (aarch64 is one example). Right? > > - Cleber. >
On Wed, Oct 10, 2018 at 08:17:26PM -0400, Cleber Rosa wrote: > > > On 10/10/18 11:47 AM, Cleber Rosa wrote: > > > > > > On 10/10/18 10:28 AM, Eduardo Habkost wrote: > >> On Wed, Oct 10, 2018 at 10:15:15AM -0400, Cleber Rosa wrote: > >>> > >>> > >>> On 10/10/18 9:59 AM, Cleber Rosa wrote: > >>>> > >>>> > >>>> On 10/10/18 9:46 AM, Eduardo Habkost wrote: > >>>>> On Wed, Oct 10, 2018 at 08:35:38AM -0400, Cleber Rosa wrote: > >>>>>> > >>>>>> > >>>>>> On 10/10/18 7:00 AM, Philippe Mathieu-Daudé wrote: > >>>>>>> On 10/10/2018 01:26, Cleber Rosa wrote: > >>>>>>>> Some targets require a machine type to be set, as there's no default > >>>>>>>> (aarch64 is one example). To give a consistent interface to users of > >>>>>>>> this API, this changes set_machine() so that a predefined default can > >>>>>>>> be used, if one is not given. The approach used is exactly the same > >>>>>>>> with the console device type. > >>>>>>>> > >>>>>>>> Also, even when there's a default machine type, for some purposes, > >>>>>>>> testing included, it's better if outside code is explicit about the > >>>>>>>> machine type, instead of relying on whatever is set internally. > >>>>>>>> > >>>>>>>> Signed-off-by: Cleber Rosa <crosa@redhat.com> > >>>>>>>> --- > >>>>>>>> scripts/qemu.py | 22 +++++++++++++++++++++- > >>>>>>>> 1 file changed, 21 insertions(+), 1 deletion(-) > >>>>>>>> > >>>>>>>> diff --git a/scripts/qemu.py b/scripts/qemu.py > >>>>>>>> index d9e24a0c1a..fca9b76990 100644 > >>>>>>>> --- a/scripts/qemu.py > >>>>>>>> +++ b/scripts/qemu.py > >>>>>>>> @@ -36,6 +36,15 @@ CONSOLE_DEV_TYPES = { > >>>>>>>> r'^s390-ccw-virtio.*': 'sclpconsole', > >>>>>>>> } > >>>>>>>> > >>>>>>>> +#: Maps archictures to the preferred machine type > >>>>>>>> +MACHINE_TYPES = { > >>>>>>>> + r'^aarch64$': 'virt', > >>>>>>>> + r'^ppc$': 'g3beige', > >>>>>>>> + r'^ppc64$': 'pseries', > >>>>>>>> + r'^s390x$': 's390-ccw-virtio', > >>>>>>>> + r'^x86_64$': 'q35', > >>>>>>> > >>>>>>> Why choose Q35 rather than PC (the default)? > >>>>>>> > >>>>>>> I was wondering about how to generate variants/machines.json but this is > >>>>>>> definitively something we want to do via a QMP query. > >>>>>>> > >>>>>>> Eduardo what do you think? > >>>>>>> > >>>>>> > >>>>>> It was motivated by Eduardo's initiative to make q35 the default "across > >>>>>> the board". He can confirm and give more details. > >>>>> > >>>>> Making Q35 the default on applications using QEMU and libvirt is > >>>>> something I'd like to happen. But I think the simplest way to do > >>>>> that is to change the QEMU default. This way you won't need this > >>>>> table on qemu.py: you can just use the default provided by QEMU. > >>>>> > >>>> > >>>> The idea is to bring consistency on how we're calling > >>>> "qemu-system-$(ARCH)", and at the same time apply the "explicit is > >>>> better than implicit" rule. > >>>> > >>>> The most important fact is that some targets do not (currently) have > >>>> "the default provided by QEMU", aarch64 is one of them. > >>>> > >>>> - Cleber. > >>>> > >>> > >>> So I ended up not relaying the question properly: should we default > >>> (even if explicitly adding "-machine") to "pc"? > >> > >> I think using the default machine-type (when QEMU has a default) > >> would be less surprising for users of the qemu.py API. > >> > > > > OK, agreed. > > > >> Implicitly adding -machine when there's no default is also > >> surprising, but then it's a nice surprise: instead of crashing > >> you get a running VM. > >> > >> Now, there are two other questions related to this: > >> > >> If using 'pc' as default, should we always add -machine, or just > >> omit the machine-type name? I think we should omit it unless the > >> caller asked for a specific machine-type name (because it would > >> be less surprising for users of the API). > >> > > > > Getting down to business, trying to apply those changes, I was faced > with a situation. Actually, the same situation I faced a few months > ago. Handling it was defered until it was *really* a blocker. > Basically the issue is: the set_console() method, which gives tests a > ready to use console, depends on knowing the machine type (see > CONSOLE_DEV_TYPES). > > As a case study, let's look at "boot_console_linux.py": > 1) it sets the machine type explicitly > 2) it has nothing to do with the specific machine type > 3) the setting of a machine type is boiler plate code to set a console > 4) the console is used on the test's real purpose: verifying the Linux > kernel booted > > Now, to be able to run the same test -- booting a Linux kernel -- on > *other target archs*, we need the same machinery. Even more important: > to have similar tests we'll need to either abstract those features or > duplicate them. This can be seen, at least in part, on the firmware > tests that Philippe sent to the list: they would also benefit from > having a console device ready to be used on the configured machine type[1]: > > Assuming that we want to provide this type of machinery for free (or as > close as that) to the acceptance/functional tests, we need some source > of "known good" configuration for the targets we aim to support. > > Let's restrict the discussion to the issue at hand, machine types, while > keeping in mind that the same pattern happened with devices types to use > as console before, and my experience running into default network device > types in further work (tests that interact with the guest by ssh'ing > into it). > > The solutions I can think of are: > > 1) run the target binary previous to the "real" run, and query > information -- this is what Avocado-VT does[2], and something I tried on > earlier versions of the acceptance tests infrastructure code > > 2) attempt to get this information from the build system[3] > > 3) hard code the "known" good configuration > > I've previously worked on solutions along the lines of #1 and #2, but > the general feedback wasn't that positive, for valid reasons. Eduardo > probably remembers this. I don't remember seeing negative feedback for #1. It sounds like the best solution. > > So, I'm tempted to try solution #3. As much as duplicating target > defaults in qemu.py doesn't sound perfect, it seems to be the more > predictable and attainable solution at this point. I consider #3 to be acceptable just as a temporary solution until we implement #1. > > Thoughts? > > Thanks! > - Cleber. > > [1] - https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg00591.html > [2] - > https://github.com/avocado-framework/avocado-vt/blob/65.0/virttest/utils_misc.py#L2105 > [3] - http://lists.gnu.org/archive/html/qemu-devel/2017-07/msg06757.html > > > OK. > > > >> About our default testing configuration for acceptance tests: > >> should acceptance tests run against PC by default? Should it > >> test Q35? Should we test both PC and Q35? I'm not sure what's > >> the answer, but I think these decisions shouldn't affect the > >> qemu.py API at all. > >> > > > > OK. > > > > To make sure we're on the same page, we're still going to have default > > machine types, based on the arch, for those targets that don't provide > > one (aarch64 is one example). Right? > > > > - Cleber. > >
On 10/10/18 11:42 PM, Eduardo Habkost wrote: > On Wed, Oct 10, 2018 at 08:17:26PM -0400, Cleber Rosa wrote: >> >> >> On 10/10/18 11:47 AM, Cleber Rosa wrote: >>> >>> >>> On 10/10/18 10:28 AM, Eduardo Habkost wrote: >>>> On Wed, Oct 10, 2018 at 10:15:15AM -0400, Cleber Rosa wrote: >>>>> >>>>> >>>>> On 10/10/18 9:59 AM, Cleber Rosa wrote: >>>>>> >>>>>> >>>>>> On 10/10/18 9:46 AM, Eduardo Habkost wrote: >>>>>>> On Wed, Oct 10, 2018 at 08:35:38AM -0400, Cleber Rosa wrote: >>>>>>>> >>>>>>>> >>>>>>>> On 10/10/18 7:00 AM, Philippe Mathieu-Daudé wrote: >>>>>>>>> On 10/10/2018 01:26, Cleber Rosa wrote: >>>>>>>>>> Some targets require a machine type to be set, as there's no default >>>>>>>>>> (aarch64 is one example). To give a consistent interface to users of >>>>>>>>>> this API, this changes set_machine() so that a predefined default can >>>>>>>>>> be used, if one is not given. The approach used is exactly the same >>>>>>>>>> with the console device type. >>>>>>>>>> >>>>>>>>>> Also, even when there's a default machine type, for some purposes, >>>>>>>>>> testing included, it's better if outside code is explicit about the >>>>>>>>>> machine type, instead of relying on whatever is set internally. >>>>>>>>>> >>>>>>>>>> Signed-off-by: Cleber Rosa <crosa@redhat.com> >>>>>>>>>> --- >>>>>>>>>> scripts/qemu.py | 22 +++++++++++++++++++++- >>>>>>>>>> 1 file changed, 21 insertions(+), 1 deletion(-) >>>>>>>>>> >>>>>>>>>> diff --git a/scripts/qemu.py b/scripts/qemu.py >>>>>>>>>> index d9e24a0c1a..fca9b76990 100644 >>>>>>>>>> --- a/scripts/qemu.py >>>>>>>>>> +++ b/scripts/qemu.py >>>>>>>>>> @@ -36,6 +36,15 @@ CONSOLE_DEV_TYPES = { >>>>>>>>>> r'^s390-ccw-virtio.*': 'sclpconsole', >>>>>>>>>> } >>>>>>>>>> >>>>>>>>>> +#: Maps archictures to the preferred machine type >>>>>>>>>> +MACHINE_TYPES = { >>>>>>>>>> + r'^aarch64$': 'virt', >>>>>>>>>> + r'^ppc$': 'g3beige', >>>>>>>>>> + r'^ppc64$': 'pseries', >>>>>>>>>> + r'^s390x$': 's390-ccw-virtio', >>>>>>>>>> + r'^x86_64$': 'q35', >>>>>>>>> >>>>>>>>> Why choose Q35 rather than PC (the default)? >>>>>>>>> >>>>>>>>> I was wondering about how to generate variants/machines.json but this is >>>>>>>>> definitively something we want to do via a QMP query. >>>>>>>>> >>>>>>>>> Eduardo what do you think? >>>>>>>>> >>>>>>>> >>>>>>>> It was motivated by Eduardo's initiative to make q35 the default "across >>>>>>>> the board". He can confirm and give more details. >>>>>>> >>>>>>> Making Q35 the default on applications using QEMU and libvirt is >>>>>>> something I'd like to happen. But I think the simplest way to do >>>>>>> that is to change the QEMU default. This way you won't need this >>>>>>> table on qemu.py: you can just use the default provided by QEMU. >>>>>>> >>>>>> >>>>>> The idea is to bring consistency on how we're calling >>>>>> "qemu-system-$(ARCH)", and at the same time apply the "explicit is >>>>>> better than implicit" rule. >>>>>> >>>>>> The most important fact is that some targets do not (currently) have >>>>>> "the default provided by QEMU", aarch64 is one of them. >>>>>> >>>>>> - Cleber. >>>>>> >>>>> >>>>> So I ended up not relaying the question properly: should we default >>>>> (even if explicitly adding "-machine") to "pc"? >>>> >>>> I think using the default machine-type (when QEMU has a default) >>>> would be less surprising for users of the qemu.py API. >>>> >>> >>> OK, agreed. >>> >>>> Implicitly adding -machine when there's no default is also >>>> surprising, but then it's a nice surprise: instead of crashing >>>> you get a running VM. >>>> >>>> Now, there are two other questions related to this: >>>> >>>> If using 'pc' as default, should we always add -machine, or just >>>> omit the machine-type name? I think we should omit it unless the >>>> caller asked for a specific machine-type name (because it would >>>> be less surprising for users of the API). >>>> >>> >> >> Getting down to business, trying to apply those changes, I was faced >> with a situation. Actually, the same situation I faced a few months >> ago. Handling it was defered until it was *really* a blocker. >> Basically the issue is: the set_console() method, which gives tests a >> ready to use console, depends on knowing the machine type (see >> CONSOLE_DEV_TYPES). >> >> As a case study, let's look at "boot_console_linux.py": >> 1) it sets the machine type explicitly >> 2) it has nothing to do with the specific machine type >> 3) the setting of a machine type is boiler plate code to set a console >> 4) the console is used on the test's real purpose: verifying the Linux >> kernel booted >> >> Now, to be able to run the same test -- booting a Linux kernel -- on >> *other target archs*, we need the same machinery. Even more important: >> to have similar tests we'll need to either abstract those features or >> duplicate them. This can be seen, at least in part, on the firmware >> tests that Philippe sent to the list: they would also benefit from >> having a console device ready to be used on the configured machine type[1]: >> >> Assuming that we want to provide this type of machinery for free (or as >> close as that) to the acceptance/functional tests, we need some source >> of "known good" configuration for the targets we aim to support. >> >> Let's restrict the discussion to the issue at hand, machine types, while >> keeping in mind that the same pattern happened with devices types to use >> as console before, and my experience running into default network device >> types in further work (tests that interact with the guest by ssh'ing >> into it). >> >> The solutions I can think of are: >> >> 1) run the target binary previous to the "real" run, and query >> information -- this is what Avocado-VT does[2], and something I tried on >> earlier versions of the acceptance tests infrastructure code >> >> 2) attempt to get this information from the build system[3] >> >> 3) hard code the "known" good configuration >> >> I've previously worked on solutions along the lines of #1 and #2, but >> the general feedback wasn't that positive, for valid reasons. Eduardo >> probably remembers this. > > I don't remember seeing negative feedback for #1. It sounds like > the best solution. > IIRC, it was mostly related to the fact that the reliable way to query a target information (instead of parsing a human oriented output) would be to use QMP. Then, the question of using QEMUMachine for that (and the possible chicken-and-egg situations, having a different set of base args, etc) led me into prototyping a simplified version: https://github.com/clebergnu/qemu/commit/b769b3d969b9e29c644d12c30cd0bb7f312e4fbc#diff-07b9b989eb85128c2c8d2a8f17685698R46 Which would still be duplicating some code. I'm not particularly happy about either approaches TBH. >> >> So, I'm tempted to try solution #3. As much as duplicating target >> defaults in qemu.py doesn't sound perfect, it seems to be the more >> predictable and attainable solution at this point. > > I consider #3 to be acceptable just as a temporary solution until > we implement #1. > > So, with the extra information given here, would you recommend doing #3? Or pause this, and work on a #1 of sorts? Thanks! - Cleber. >> >> Thoughts? >> >> Thanks! >> - Cleber. >> >> [1] - https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg00591.html >> [2] - >> https://github.com/avocado-framework/avocado-vt/blob/65.0/virttest/utils_misc.py#L2105 >> [3] - http://lists.gnu.org/archive/html/qemu-devel/2017-07/msg06757.html >> >>> OK. >>> >>>> About our default testing configuration for acceptance tests: >>>> should acceptance tests run against PC by default? Should it >>>> test Q35? Should we test both PC and Q35? I'm not sure what's >>>> the answer, but I think these decisions shouldn't affect the >>>> qemu.py API at all. >>>> >>> >>> OK. >>> >>> To make sure we're on the same page, we're still going to have default >>> machine types, based on the arch, for those targets that don't provide >>> one (aarch64 is one example). Right? >>> >>> - Cleber. >>> >
On Thu, Oct 11, 2018 at 12:43:06AM -0400, Cleber Rosa wrote: > > > On 10/10/18 11:42 PM, Eduardo Habkost wrote: > > On Wed, Oct 10, 2018 at 08:17:26PM -0400, Cleber Rosa wrote: > >> > >> > >> On 10/10/18 11:47 AM, Cleber Rosa wrote: > >>> > >>> > >>> On 10/10/18 10:28 AM, Eduardo Habkost wrote: > >>>> On Wed, Oct 10, 2018 at 10:15:15AM -0400, Cleber Rosa wrote: > >>>>> > >>>>> > >>>>> On 10/10/18 9:59 AM, Cleber Rosa wrote: > >>>>>> > >>>>>> > >>>>>> On 10/10/18 9:46 AM, Eduardo Habkost wrote: > >>>>>>> On Wed, Oct 10, 2018 at 08:35:38AM -0400, Cleber Rosa wrote: > >>>>>>>> > >>>>>>>> > >>>>>>>> On 10/10/18 7:00 AM, Philippe Mathieu-Daudé wrote: > >>>>>>>>> On 10/10/2018 01:26, Cleber Rosa wrote: > >>>>>>>>>> Some targets require a machine type to be set, as there's no default > >>>>>>>>>> (aarch64 is one example). To give a consistent interface to users of > >>>>>>>>>> this API, this changes set_machine() so that a predefined default can > >>>>>>>>>> be used, if one is not given. The approach used is exactly the same > >>>>>>>>>> with the console device type. > >>>>>>>>>> > >>>>>>>>>> Also, even when there's a default machine type, for some purposes, > >>>>>>>>>> testing included, it's better if outside code is explicit about the > >>>>>>>>>> machine type, instead of relying on whatever is set internally. > >>>>>>>>>> > >>>>>>>>>> Signed-off-by: Cleber Rosa <crosa@redhat.com> > >>>>>>>>>> --- > >>>>>>>>>> scripts/qemu.py | 22 +++++++++++++++++++++- > >>>>>>>>>> 1 file changed, 21 insertions(+), 1 deletion(-) > >>>>>>>>>> > >>>>>>>>>> diff --git a/scripts/qemu.py b/scripts/qemu.py > >>>>>>>>>> index d9e24a0c1a..fca9b76990 100644 > >>>>>>>>>> --- a/scripts/qemu.py > >>>>>>>>>> +++ b/scripts/qemu.py > >>>>>>>>>> @@ -36,6 +36,15 @@ CONSOLE_DEV_TYPES = { > >>>>>>>>>> r'^s390-ccw-virtio.*': 'sclpconsole', > >>>>>>>>>> } > >>>>>>>>>> > >>>>>>>>>> +#: Maps archictures to the preferred machine type > >>>>>>>>>> +MACHINE_TYPES = { > >>>>>>>>>> + r'^aarch64$': 'virt', > >>>>>>>>>> + r'^ppc$': 'g3beige', > >>>>>>>>>> + r'^ppc64$': 'pseries', > >>>>>>>>>> + r'^s390x$': 's390-ccw-virtio', > >>>>>>>>>> + r'^x86_64$': 'q35', > >>>>>>>>> > >>>>>>>>> Why choose Q35 rather than PC (the default)? > >>>>>>>>> > >>>>>>>>> I was wondering about how to generate variants/machines.json but this is > >>>>>>>>> definitively something we want to do via a QMP query. > >>>>>>>>> > >>>>>>>>> Eduardo what do you think? > >>>>>>>>> > >>>>>>>> > >>>>>>>> It was motivated by Eduardo's initiative to make q35 the default "across > >>>>>>>> the board". He can confirm and give more details. > >>>>>>> > >>>>>>> Making Q35 the default on applications using QEMU and libvirt is > >>>>>>> something I'd like to happen. But I think the simplest way to do > >>>>>>> that is to change the QEMU default. This way you won't need this > >>>>>>> table on qemu.py: you can just use the default provided by QEMU. > >>>>>>> > >>>>>> > >>>>>> The idea is to bring consistency on how we're calling > >>>>>> "qemu-system-$(ARCH)", and at the same time apply the "explicit is > >>>>>> better than implicit" rule. > >>>>>> > >>>>>> The most important fact is that some targets do not (currently) have > >>>>>> "the default provided by QEMU", aarch64 is one of them. > >>>>>> > >>>>>> - Cleber. > >>>>>> > >>>>> > >>>>> So I ended up not relaying the question properly: should we default > >>>>> (even if explicitly adding "-machine") to "pc"? > >>>> > >>>> I think using the default machine-type (when QEMU has a default) > >>>> would be less surprising for users of the qemu.py API. > >>>> > >>> > >>> OK, agreed. > >>> > >>>> Implicitly adding -machine when there's no default is also > >>>> surprising, but then it's a nice surprise: instead of crashing > >>>> you get a running VM. > >>>> > >>>> Now, there are two other questions related to this: > >>>> > >>>> If using 'pc' as default, should we always add -machine, or just > >>>> omit the machine-type name? I think we should omit it unless the > >>>> caller asked for a specific machine-type name (because it would > >>>> be less surprising for users of the API). > >>>> > >>> > >> > >> Getting down to business, trying to apply those changes, I was faced > >> with a situation. Actually, the same situation I faced a few months > >> ago. Handling it was defered until it was *really* a blocker. > >> Basically the issue is: the set_console() method, which gives tests a > >> ready to use console, depends on knowing the machine type (see > >> CONSOLE_DEV_TYPES). > >> > >> As a case study, let's look at "boot_console_linux.py": > >> 1) it sets the machine type explicitly > >> 2) it has nothing to do with the specific machine type > >> 3) the setting of a machine type is boiler plate code to set a console > >> 4) the console is used on the test's real purpose: verifying the Linux > >> kernel booted > >> > >> Now, to be able to run the same test -- booting a Linux kernel -- on > >> *other target archs*, we need the same machinery. Even more important: > >> to have similar tests we'll need to either abstract those features or > >> duplicate them. This can be seen, at least in part, on the firmware > >> tests that Philippe sent to the list: they would also benefit from > >> having a console device ready to be used on the configured machine type[1]: > >> > >> Assuming that we want to provide this type of machinery for free (or as > >> close as that) to the acceptance/functional tests, we need some source > >> of "known good" configuration for the targets we aim to support. > >> > >> Let's restrict the discussion to the issue at hand, machine types, while > >> keeping in mind that the same pattern happened with devices types to use > >> as console before, and my experience running into default network device > >> types in further work (tests that interact with the guest by ssh'ing > >> into it). > >> > >> The solutions I can think of are: > >> > >> 1) run the target binary previous to the "real" run, and query > >> information -- this is what Avocado-VT does[2], and something I tried on > >> earlier versions of the acceptance tests infrastructure code > >> > >> 2) attempt to get this information from the build system[3] > >> > >> 3) hard code the "known" good configuration > >> > >> I've previously worked on solutions along the lines of #1 and #2, but > >> the general feedback wasn't that positive, for valid reasons. Eduardo > >> probably remembers this. > > > > I don't remember seeing negative feedback for #1. It sounds like > > the best solution. > > > > IIRC, it was mostly related to the fact that the reliable way to query a > target information (instead of parsing a human oriented output) would be > to use QMP. > > Then, the question of using QEMUMachine for that (and the possible > chicken-and-egg situations, having a different set of base args, etc) > led me into prototyping a simplified version: > > https://github.com/clebergnu/qemu/commit/b769b3d969b9e29c644d12c30cd0bb7f312e4fbc#diff-07b9b989eb85128c2c8d2a8f17685698R46 > > Which would still be duplicating some code. I'm not particularly happy > about either approaches TBH. > > >> > >> So, I'm tempted to try solution #3. As much as duplicating target > >> defaults in qemu.py doesn't sound perfect, it seems to be the more > >> predictable and attainable solution at this point. > > > > I consider #3 to be acceptable just as a temporary solution until > > we implement #1. > > > > > > So, with the extra information given here, would you recommend doing #3? > Or pause this, and work on a #1 of sorts? Getting more working tests is more important to me. I won't mind implementing #3 on the avocado_qemu module if implementing #1 first would delay the inclusion of tests on QEMU 3.1.
On 10 October 2018 at 20:54, Cleber Rosa <crosa@redhat.com> wrote: > On 10/10/18 2:07 PM, Peter Maydell wrote: >> We should aim to test the machines we care about (regardless >> of what architectures they are), rather than thinking about it >> in terms of "testing architectures X, Y, Z", I think. >> > > To me it's clear that: > > 1) I lack a complete understanding of what "we care about" > 2) It's easier to start with something, and tweak it to taste Right. Effectively you're already defining a set of "machines we care about" -- it's just that rather than explicitly saying 'we care about x86-64 pc, i386 pc, aarch64 virt, ppc spapr, ...' you're implicitly doing it by defining a 'default machine' for various architectures and a list of architectures. I think you should just list the machines you're testing with, instead. I don't mind if we start with a list of what we're testing that's small (or even wrong!) -- we can expand and correct it later. I'd just like the list to be the correct shape. thanks -- PMM
diff --git a/scripts/qemu.py b/scripts/qemu.py index d9e24a0c1a..fca9b76990 100644 --- a/scripts/qemu.py +++ b/scripts/qemu.py @@ -36,6 +36,15 @@ CONSOLE_DEV_TYPES = { r'^s390-ccw-virtio.*': 'sclpconsole', } +#: Maps archictures to the preferred machine type +MACHINE_TYPES = { + r'^aarch64$': 'virt', + r'^ppc$': 'g3beige', + r'^ppc64$': 'pseries', + r'^s390x$': 's390-ccw-virtio', + r'^x86_64$': 'q35', + } + class QEMUMachineError(Exception): """ @@ -413,13 +422,24 @@ class QEMUMachine(object): """ self._arch = arch - def set_machine(self, machine_type): + def set_machine(self, machine_type=None): ''' Sets the machine type If set, the machine type will be added to the base arguments of the resulting QEMU command line. ''' + if machine_type is None: + if self._arch is None: + raise QEMUMachineError("Can not set a default machine type: " + "QEMU instance without a defined arch") + for regex, machine in MACHINE_TYPES.items(): + if re.match(regex, self._arch): + machine_type = machine + break + if machine_type is None: + raise QEMUMachineError("Can not set a machine type: no " + "matching machine type definition") self._machine = machine_type def set_console(self, device_type=None):
Some targets require a machine type to be set, as there's no default (aarch64 is one example). To give a consistent interface to users of this API, this changes set_machine() so that a predefined default can be used, if one is not given. The approach used is exactly the same with the console device type. Also, even when there's a default machine type, for some purposes, testing included, it's better if outside code is explicit about the machine type, instead of relying on whatever is set internally. Signed-off-by: Cleber Rosa <crosa@redhat.com> --- scripts/qemu.py | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-)