| Message ID | 20191218170003.31356-2-wainersm@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Acceptance tests: boot Linux with KVM test | expand |
On 18/12/2019 18.00, Wainer dos Santos Moschetta wrote: > The test case may need to boot the VM with an accelerator that > isn't actually enabled on the QEMU binary and/or present in the host. In > this case the test behavior is undefined, and the best course of > action is to skip its execution. > > This change introduced the 'accel' parameter (and the handler of > tag with same name) used to indicate the test case requires a > given accelerator available. It was implemented a mechanism to > skip the test case if the accelerator is not available. Moreover, > the QEMU --accel argument is set automatically to any VM > launched if the parameter is present. > > Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com> > --- > docs/devel/testing.rst | 16 ++++++++++++++++ > tests/acceptance/avocado_qemu/__init__.py | 23 +++++++++++++++++++++++ > 2 files changed, 39 insertions(+) > > diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst > index 27f286858a..6c2e0718e1 100644 > --- a/docs/devel/testing.rst > +++ b/docs/devel/testing.rst > @@ -757,6 +757,17 @@ name. If one is not given explicitly, it will either be set to > ``None``, or, if the test is tagged with one (and only one) > ``:avocado: tags=machine:VALUE`` tag, it will be set to ``VALUE``. > > +accel > +~~~~~ > +The accelerator that will be set to all QEMUMachine instances created > +by the test. > + > +The ``accel`` attribute will be set to the test parameter of the same > +name. If one is not given explicitly, it will either be set to > +``None``, or, if the test is tagged with one (and only one) > +``:avocado: tags=accel:VALUE`` tag, it will be set to ``VALUE``. Currently > +``VALUE`` should be either ``kvm`` or ``tcg``. > + > qemu_bin > ~~~~~~~~ > > @@ -798,6 +809,11 @@ machine > The machine type that will be set to all QEMUMachine instances created > by the test. > > +accel > +~~~~~ > +The accelerator that will be set to all QEMUMachine instances created > +by the test. In case the accelerator is not available (both QEMU > +binary and the host system are checked) then the test is canceled. > > qemu_bin > ~~~~~~~~ > diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py > index 6618ea67c1..aff32668d9 100644 > --- a/tests/acceptance/avocado_qemu/__init__.py > +++ b/tests/acceptance/avocado_qemu/__init__.py > @@ -20,6 +20,7 @@ SRC_ROOT_DIR = os.path.join(os.path.dirname(__file__), '..', '..', '..') > sys.path.append(os.path.join(SRC_ROOT_DIR, 'python')) > > from qemu.machine import QEMUMachine > +from qemu.accel import kvm_available, tcg_available > > def is_readable_executable_file(path): > return os.path.isfile(path) and os.access(path, os.R_OK | os.X_OK) > @@ -111,6 +112,8 @@ class Test(avocado.Test): > > def setUp(self): > self._vms = {} > + # VM argumments that are mapped from parameters > + self._param_to_vm_args = [] > > self.arch = self.params.get('arch', > default=self._get_unique_tag_val('arch')) > @@ -124,10 +127,30 @@ class Test(avocado.Test): > if self.qemu_bin is None: > self.cancel("No QEMU binary defined or found in the source tree") > > + self.accel = self.params.get('accel', > + default=self._get_unique_tag_val('accel')) > + if self.accel: > + avail = False > + if self.accel == 'kvm': > + if kvm_available(self.arch, self.qemu_bin): > + self._param_to_vm_args.append('-enable-kvm') Could you please use "-accel kvm" instead? "-accel" is now our official way to configure an accelerator ... so we should not use the old wrappers in new code anymore if possible. Thanks, Thomas PS: Travis supports KVM now, too (with some tweaking of the permissions) ... maybe we should now try to get some QEMU tests running with KVM there, too...
Hi Thomas, On 12/18/19 4:48 PM, Thomas Huth wrote: > On 18/12/2019 18.00, Wainer dos Santos Moschetta wrote: >> The test case may need to boot the VM with an accelerator that >> isn't actually enabled on the QEMU binary and/or present in the host. In >> this case the test behavior is undefined, and the best course of >> action is to skip its execution. >> >> This change introduced the 'accel' parameter (and the handler of >> tag with same name) used to indicate the test case requires a >> given accelerator available. It was implemented a mechanism to >> skip the test case if the accelerator is not available. Moreover, >> the QEMU --accel argument is set automatically to any VM >> launched if the parameter is present. >> >> Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com> >> --- >> docs/devel/testing.rst | 16 ++++++++++++++++ >> tests/acceptance/avocado_qemu/__init__.py | 23 +++++++++++++++++++++++ >> 2 files changed, 39 insertions(+) >> >> diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst >> index 27f286858a..6c2e0718e1 100644 >> --- a/docs/devel/testing.rst >> +++ b/docs/devel/testing.rst >> @@ -757,6 +757,17 @@ name. If one is not given explicitly, it will either be set to >> ``None``, or, if the test is tagged with one (and only one) >> ``:avocado: tags=machine:VALUE`` tag, it will be set to ``VALUE``. >> >> +accel >> +~~~~~ >> +The accelerator that will be set to all QEMUMachine instances created >> +by the test. >> + >> +The ``accel`` attribute will be set to the test parameter of the same >> +name. If one is not given explicitly, it will either be set to >> +``None``, or, if the test is tagged with one (and only one) >> +``:avocado: tags=accel:VALUE`` tag, it will be set to ``VALUE``. Currently >> +``VALUE`` should be either ``kvm`` or ``tcg``. >> + >> qemu_bin >> ~~~~~~~~ >> >> @@ -798,6 +809,11 @@ machine >> The machine type that will be set to all QEMUMachine instances created >> by the test. >> >> +accel >> +~~~~~ >> +The accelerator that will be set to all QEMUMachine instances created >> +by the test. In case the accelerator is not available (both QEMU >> +binary and the host system are checked) then the test is canceled. >> >> qemu_bin >> ~~~~~~~~ >> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py >> index 6618ea67c1..aff32668d9 100644 >> --- a/tests/acceptance/avocado_qemu/__init__.py >> +++ b/tests/acceptance/avocado_qemu/__init__.py >> @@ -20,6 +20,7 @@ SRC_ROOT_DIR = os.path.join(os.path.dirname(__file__), '..', '..', '..') >> sys.path.append(os.path.join(SRC_ROOT_DIR, 'python')) >> >> from qemu.machine import QEMUMachine >> +from qemu.accel import kvm_available, tcg_available >> >> def is_readable_executable_file(path): >> return os.path.isfile(path) and os.access(path, os.R_OK | os.X_OK) >> @@ -111,6 +112,8 @@ class Test(avocado.Test): >> >> def setUp(self): >> self._vms = {} >> + # VM argumments that are mapped from parameters >> + self._param_to_vm_args = [] >> >> self.arch = self.params.get('arch', >> default=self._get_unique_tag_val('arch')) >> @@ -124,10 +127,30 @@ class Test(avocado.Test): >> if self.qemu_bin is None: >> self.cancel("No QEMU binary defined or found in the source tree") >> >> + self.accel = self.params.get('accel', >> + default=self._get_unique_tag_val('accel')) >> + if self.accel: >> + avail = False >> + if self.accel == 'kvm': >> + if kvm_available(self.arch, self.qemu_bin): >> + self._param_to_vm_args.append('-enable-kvm') > Could you please use "-accel kvm" instead? "-accel" is now our official > way to configure an accelerator ... so we should not use the old > wrappers in new code anymore if possible. Sure, I am going to adjust that on v3. > > Thanks, > Thomas > > > PS: Travis supports KVM now, too (with some tweaking of the permissions) > ... maybe we should now try to get some QEMU tests running with KVM > there, too... I heard that but I failed miserably to enable nested virt on Travis. Actually I was expecting it enabled by default but not the case. I did not find documentation so I tried some tweaks like setting 'sudo:required' and using bionic but none of that worked out. Do you know what needs to be done? Thanks! - Wainer
On 10/01/2020 21.02, Wainer dos Santos Moschetta wrote: > Hi Thomas, > > On 12/18/19 4:48 PM, Thomas Huth wrote: >> On 18/12/2019 18.00, Wainer dos Santos Moschetta wrote: >>> The test case may need to boot the VM with an accelerator that >>> isn't actually enabled on the QEMU binary and/or present in the host. In >>> this case the test behavior is undefined, and the best course of >>> action is to skip its execution. >>> >>> This change introduced the 'accel' parameter (and the handler of >>> tag with same name) used to indicate the test case requires a >>> given accelerator available. It was implemented a mechanism to >>> skip the test case if the accelerator is not available. Moreover, >>> the QEMU --accel argument is set automatically to any VM >>> launched if the parameter is present. >>> >>> Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com> >>> --- >>> docs/devel/testing.rst | 16 ++++++++++++++++ >>> tests/acceptance/avocado_qemu/__init__.py | 23 +++++++++++++++++++++++ >>> 2 files changed, 39 insertions(+) >>> >>> diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst >>> index 27f286858a..6c2e0718e1 100644 >>> --- a/docs/devel/testing.rst >>> +++ b/docs/devel/testing.rst >>> @@ -757,6 +757,17 @@ name. If one is not given explicitly, it will >>> either be set to >>> ``None``, or, if the test is tagged with one (and only one) >>> ``:avocado: tags=machine:VALUE`` tag, it will be set to ``VALUE``. >>> +accel >>> +~~~~~ >>> +The accelerator that will be set to all QEMUMachine instances created >>> +by the test. >>> + >>> +The ``accel`` attribute will be set to the test parameter of the same >>> +name. If one is not given explicitly, it will either be set to >>> +``None``, or, if the test is tagged with one (and only one) >>> +``:avocado: tags=accel:VALUE`` tag, it will be set to ``VALUE``. >>> Currently >>> +``VALUE`` should be either ``kvm`` or ``tcg``. >>> + >>> qemu_bin >>> ~~~~~~~~ >>> @@ -798,6 +809,11 @@ machine >>> The machine type that will be set to all QEMUMachine instances created >>> by the test. >>> +accel >>> +~~~~~ >>> +The accelerator that will be set to all QEMUMachine instances created >>> +by the test. In case the accelerator is not available (both QEMU >>> +binary and the host system are checked) then the test is canceled. >>> qemu_bin >>> ~~~~~~~~ >>> diff --git a/tests/acceptance/avocado_qemu/__init__.py >>> b/tests/acceptance/avocado_qemu/__init__.py >>> index 6618ea67c1..aff32668d9 100644 >>> --- a/tests/acceptance/avocado_qemu/__init__.py >>> +++ b/tests/acceptance/avocado_qemu/__init__.py >>> @@ -20,6 +20,7 @@ SRC_ROOT_DIR = >>> os.path.join(os.path.dirname(__file__), '..', '..', '..') >>> sys.path.append(os.path.join(SRC_ROOT_DIR, 'python')) >>> from qemu.machine import QEMUMachine >>> +from qemu.accel import kvm_available, tcg_available >>> def is_readable_executable_file(path): >>> return os.path.isfile(path) and os.access(path, os.R_OK | os.X_OK) >>> @@ -111,6 +112,8 @@ class Test(avocado.Test): >>> def setUp(self): >>> self._vms = {} >>> + # VM argumments that are mapped from parameters >>> + self._param_to_vm_args = [] >>> self.arch = self.params.get('arch', >>> >>> default=self._get_unique_tag_val('arch')) >>> @@ -124,10 +127,30 @@ class Test(avocado.Test): >>> if self.qemu_bin is None: >>> self.cancel("No QEMU binary defined or found in the >>> source tree") >>> + self.accel = self.params.get('accel', >>> + >>> default=self._get_unique_tag_val('accel')) >>> + if self.accel: >>> + avail = False >>> + if self.accel == 'kvm': >>> + if kvm_available(self.arch, self.qemu_bin): >>> + self._param_to_vm_args.append('-enable-kvm') >> Could you please use "-accel kvm" instead? "-accel" is now our official >> way to configure an accelerator ... so we should not use the old >> wrappers in new code anymore if possible. > Sure, I am going to adjust that on v3. >> >> Thanks, >> Thomas >> >> >> PS: Travis supports KVM now, too (with some tweaking of the permissions) >> ... maybe we should now try to get some QEMU tests running with KVM >> there, too... > > I heard that but I failed miserably to enable nested virt on Travis. > Actually I was expecting it enabled by default but not the case. I did > not find documentation so I tried some tweaks like setting > 'sudo:required' and using bionic but none of that worked out. > > Do you know what needs to be done? Yes, I recently enabled it for the kvm-unit-tests ... and yes, it's a bit ugly: The user has to be in the "kvm" group which is not the case for the user that runs the travis scripts. Tweaking the access rights of /dev/kvm unfortunately does not work (at least not directly via chmod o+rwx /dev/kvm ... but maybe there is a way via udev or something similar?), so I ended up with: sudo chgrp kvm /usr/bin/qemu-system-* sudo chmod g+s /usr/bin/qemu-system-* With that, the kvm-unit-tests are running now fine with KVM on Travis. Thomas
On 1/11/20 6:56 AM, Thomas Huth wrote: > On 10/01/2020 21.02, Wainer dos Santos Moschetta wrote: >> Hi Thomas, >> >> On 12/18/19 4:48 PM, Thomas Huth wrote: >>> On 18/12/2019 18.00, Wainer dos Santos Moschetta wrote: >>>> The test case may need to boot the VM with an accelerator that >>>> isn't actually enabled on the QEMU binary and/or present in the host. In >>>> this case the test behavior is undefined, and the best course of >>>> action is to skip its execution. >>>> >>>> This change introduced the 'accel' parameter (and the handler of >>>> tag with same name) used to indicate the test case requires a >>>> given accelerator available. It was implemented a mechanism to >>>> skip the test case if the accelerator is not available. Moreover, >>>> the QEMU --accel argument is set automatically to any VM >>>> launched if the parameter is present. >>>> >>>> Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com> >>>> --- >>>> docs/devel/testing.rst | 16 ++++++++++++++++ >>>> tests/acceptance/avocado_qemu/__init__.py | 23 +++++++++++++++++++++++ >>>> 2 files changed, 39 insertions(+) >>>> >>>> diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst >>>> index 27f286858a..6c2e0718e1 100644 >>>> --- a/docs/devel/testing.rst >>>> +++ b/docs/devel/testing.rst >>>> @@ -757,6 +757,17 @@ name. If one is not given explicitly, it will >>>> either be set to >>>> ``None``, or, if the test is tagged with one (and only one) >>>> ``:avocado: tags=machine:VALUE`` tag, it will be set to ``VALUE``. >>>> +accel >>>> +~~~~~ >>>> +The accelerator that will be set to all QEMUMachine instances created >>>> +by the test. >>>> + >>>> +The ``accel`` attribute will be set to the test parameter of the same >>>> +name. If one is not given explicitly, it will either be set to >>>> +``None``, or, if the test is tagged with one (and only one) >>>> +``:avocado: tags=accel:VALUE`` tag, it will be set to ``VALUE``. >>>> Currently >>>> +``VALUE`` should be either ``kvm`` or ``tcg``. >>>> + >>>> qemu_bin >>>> ~~~~~~~~ >>>> @@ -798,6 +809,11 @@ machine >>>> The machine type that will be set to all QEMUMachine instances created >>>> by the test. >>>> +accel >>>> +~~~~~ >>>> +The accelerator that will be set to all QEMUMachine instances created >>>> +by the test. In case the accelerator is not available (both QEMU >>>> +binary and the host system are checked) then the test is canceled. >>>> qemu_bin >>>> ~~~~~~~~ >>>> diff --git a/tests/acceptance/avocado_qemu/__init__.py >>>> b/tests/acceptance/avocado_qemu/__init__.py >>>> index 6618ea67c1..aff32668d9 100644 >>>> --- a/tests/acceptance/avocado_qemu/__init__.py >>>> +++ b/tests/acceptance/avocado_qemu/__init__.py >>>> @@ -20,6 +20,7 @@ SRC_ROOT_DIR = >>>> os.path.join(os.path.dirname(__file__), '..', '..', '..') >>>> sys.path.append(os.path.join(SRC_ROOT_DIR, 'python')) >>>> from qemu.machine import QEMUMachine >>>> +from qemu.accel import kvm_available, tcg_available >>>> def is_readable_executable_file(path): >>>> return os.path.isfile(path) and os.access(path, os.R_OK | os.X_OK) >>>> @@ -111,6 +112,8 @@ class Test(avocado.Test): >>>> def setUp(self): >>>> self._vms = {} >>>> + # VM argumments that are mapped from parameters >>>> + self._param_to_vm_args = [] >>>> self.arch = self.params.get('arch', >>>> >>>> default=self._get_unique_tag_val('arch')) >>>> @@ -124,10 +127,30 @@ class Test(avocado.Test): >>>> if self.qemu_bin is None: >>>> self.cancel("No QEMU binary defined or found in the >>>> source tree") >>>> + self.accel = self.params.get('accel', >>>> + >>>> default=self._get_unique_tag_val('accel')) >>>> + if self.accel: >>>> + avail = False >>>> + if self.accel == 'kvm': >>>> + if kvm_available(self.arch, self.qemu_bin): >>>> + self._param_to_vm_args.append('-enable-kvm') >>> Could you please use "-accel kvm" instead? "-accel" is now our official >>> way to configure an accelerator ... so we should not use the old >>> wrappers in new code anymore if possible. >> Sure, I am going to adjust that on v3. >>> Thanks, >>> Thomas >>> >>> >>> PS: Travis supports KVM now, too (with some tweaking of the permissions) >>> ... maybe we should now try to get some QEMU tests running with KVM >>> there, too... >> I heard that but I failed miserably to enable nested virt on Travis. >> Actually I was expecting it enabled by default but not the case. I did >> not find documentation so I tried some tweaks like setting >> 'sudo:required' and using bionic but none of that worked out. >> >> Do you know what needs to be done? > Yes, I recently enabled it for the kvm-unit-tests ... and yes, it's a > bit ugly: The user has to be in the "kvm" group which is not the case > for the user that runs the travis scripts. Tweaking the access rights of > /dev/kvm unfortunately does not work (at least not directly via chmod > o+rwx /dev/kvm ... but maybe there is a way via udev or something > similar?), so I ended up with: > > sudo chgrp kvm /usr/bin/qemu-system-* > sudo chmod g+s /usr/bin/qemu-system-* > > With that, the kvm-unit-tests are running now fine with KVM on Travis. Hi Thomas, Thanks for pointing out how you did setup Travis+KVM on kvm-unit-tests. I just sent a v3. You can see that I changed the Travis's acceptance tests builder to run on Ubuntu Bionic, and tweaked the access rights (chmod o+rw /dev/kvm) which, unlike on kvm-unit-tests, it is enough to make KVM available for the acceptance tests. - Wainer > > Thomas
diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst index 27f286858a..6c2e0718e1 100644 --- a/docs/devel/testing.rst +++ b/docs/devel/testing.rst @@ -757,6 +757,17 @@ name. If one is not given explicitly, it will either be set to ``None``, or, if the test is tagged with one (and only one) ``:avocado: tags=machine:VALUE`` tag, it will be set to ``VALUE``. +accel +~~~~~ +The accelerator that will be set to all QEMUMachine instances created +by the test. + +The ``accel`` attribute will be set to the test parameter of the same +name. If one is not given explicitly, it will either be set to +``None``, or, if the test is tagged with one (and only one) +``:avocado: tags=accel:VALUE`` tag, it will be set to ``VALUE``. Currently +``VALUE`` should be either ``kvm`` or ``tcg``. + qemu_bin ~~~~~~~~ @@ -798,6 +809,11 @@ machine The machine type that will be set to all QEMUMachine instances created by the test. +accel +~~~~~ +The accelerator that will be set to all QEMUMachine instances created +by the test. In case the accelerator is not available (both QEMU +binary and the host system are checked) then the test is canceled. qemu_bin ~~~~~~~~ diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py index 6618ea67c1..aff32668d9 100644 --- a/tests/acceptance/avocado_qemu/__init__.py +++ b/tests/acceptance/avocado_qemu/__init__.py @@ -20,6 +20,7 @@ SRC_ROOT_DIR = os.path.join(os.path.dirname(__file__), '..', '..', '..') sys.path.append(os.path.join(SRC_ROOT_DIR, 'python')) from qemu.machine import QEMUMachine +from qemu.accel import kvm_available, tcg_available def is_readable_executable_file(path): return os.path.isfile(path) and os.access(path, os.R_OK | os.X_OK) @@ -111,6 +112,8 @@ class Test(avocado.Test): def setUp(self): self._vms = {} + # VM argumments that are mapped from parameters + self._param_to_vm_args = [] self.arch = self.params.get('arch', default=self._get_unique_tag_val('arch')) @@ -124,10 +127,30 @@ class Test(avocado.Test): if self.qemu_bin is None: self.cancel("No QEMU binary defined or found in the source tree") + self.accel = self.params.get('accel', + default=self._get_unique_tag_val('accel')) + if self.accel: + avail = False + if self.accel == 'kvm': + if kvm_available(self.arch, self.qemu_bin): + self._param_to_vm_args.append('-enable-kvm') + avail = True + elif self.accel == 'tcg': + if tcg_available(self.qemu_bin): + self._param_to_vm_args.extend(['--accel', 'tcg']) + avail = True + else: + self.cancel("Unknown accelerator: %s" % self.accel) + + if not avail: + self.cancel("%s is not available" % self.accel) + def _new_vm(self, *args): vm = QEMUMachine(self.qemu_bin, sock_dir=tempfile.mkdtemp()) if args: vm.add_args(*args) + if self._param_to_vm_args: + vm.add_args(*self._param_to_vm_args) return vm @property
The test case may need to boot the VM with an accelerator that isn't actually enabled on the QEMU binary and/or present in the host. In this case the test behavior is undefined, and the best course of action is to skip its execution. This change introduced the 'accel' parameter (and the handler of tag with same name) used to indicate the test case requires a given accelerator available. It was implemented a mechanism to skip the test case if the accelerator is not available. Moreover, the QEMU --accel argument is set automatically to any VM launched if the parameter is present. Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com> --- docs/devel/testing.rst | 16 ++++++++++++++++ tests/acceptance/avocado_qemu/__init__.py | 23 +++++++++++++++++++++++ 2 files changed, 39 insertions(+)