| Message ID | 20230228192628.26140-6-farosas@suse.de |
|---|---|
| State | New |
| Headers | show |
| Series | target/arm: Allow CONFIG_TCG=n builds | expand |
On Tue, 28 Feb 2023 at 19:28, Fabiano Rosas <farosas@suse.de> wrote: > > The migration tests are currently broken for an aarch64 host because > the tests pass no 'machine' and 'cpu' options on the QEMU command > line. > > Add a separate class to each architecture so that we can specify > 'machine' and 'cpu' options instead of relying on defaults. > > Add a skip decorator to keep the current behavior of only running > migration tests when the qemu target matches the host architecture. I still don't understand this patch. Don't we run the migration-test on all hosts already? David ? > Signed-off-by: Fabiano Rosas <farosas@suse.de> > --- > tests/avocado/migration.py | 83 +++++++++++++++++++++++++++++++++++--- > 1 file changed, 78 insertions(+), 5 deletions(-) > > diff --git a/tests/avocado/migration.py b/tests/avocado/migration.py > index 4b25680c50..8b2ec0e3c4 100644 > --- a/tests/avocado/migration.py > +++ b/tests/avocado/migration.py > @@ -11,6 +11,8 @@ > > > import tempfile > +import os > + > from avocado_qemu import QemuSystemTest > from avocado import skipUnless > > @@ -19,7 +21,7 @@ > from avocado.utils.path import find_command > > > -class Migration(QemuSystemTest): > +class MigrationTest(QemuSystemTest): > """ > :avocado: tags=migration > """ > @@ -62,20 +64,91 @@ def _get_free_port(self): > self.cancel('Failed to find a free port') > return port > > - > - def test_migration_with_tcp_localhost(self): > + def migration_with_tcp_localhost(self): > dest_uri = 'tcp:localhost:%u' % self._get_free_port() > self.do_migrate(dest_uri) > > - def test_migration_with_unix(self): > + def migration_with_unix(self): > with tempfile.TemporaryDirectory(prefix='socket_') as socket_path: > dest_uri = 'unix:%s/qemu-test.sock' % socket_path > self.do_migrate(dest_uri) > > @skipUnless(find_command('nc', default=False), "'nc' command not found") > - def test_migration_with_exec(self): > + def migration_with_exec(self): > """The test works for both netcat-traditional and netcat-openbsd packages.""" > free_port = self._get_free_port() > dest_uri = 'exec:nc -l localhost %u' % free_port > src_uri = 'exec:nc localhost %u' % free_port > self.do_migrate(dest_uri, src_uri) > + > + > +@skipUnless('aarch64' in os.uname()[4], "host != target") > +class Aarch64(MigrationTest): > + """ > + :avocado: tags=arch:aarch64 > + :avocado: tags=machine:virt > + :avocado: tags=cpu:max > + """ > + > + def test_migration_with_tcp_localhost(self): > + self.migration_with_tcp_localhost() > + > + def test_migration_with_unix(self): > + self.migration_with_unix() > + > + def test_migration_with_exec(self): > + self.migration_with_exec() > + > + > +@skipUnless('x86_64' in os.uname()[4], "host != target") > +class X86_64(MigrationTest): > + """ > + :avocado: tags=arch:x86_64 > + :avocado: tags=machine:pc > + :avocado: tags=cpu:qemu64 > + """ > + > + def test_migration_with_tcp_localhost(self): > + self.migration_with_tcp_localhost() > + > + def test_migration_with_unix(self): > + self.migration_with_unix() > + > + def test_migration_with_exec(self): > + self.migration_with_exec() > + > + > +@skipUnless('ppc64le' in os.uname()[4], "host != target") > +class PPC64(MigrationTest): > + """ > + :avocado: tags=arch:ppc64 > + :avocado: tags=machine:pseries > + :avocado: tags=cpu:power9_v2.0 > + """ > + > + def test_migration_with_tcp_localhost(self): > + self.migration_with_tcp_localhost() > + > + def test_migration_with_unix(self): > + self.migration_with_unix() > + > + def test_migration_with_exec(self): > + self.migration_with_exec() > + > + > +@skipUnless('s390x' in os.uname()[4], "host != target") > +class S390X(MigrationTest): > + """ > + :avocado: tags=arch:s390x > + :avocado: tags=machine:s390-ccw-virtio > + :avocado: tags=cpu:qemu > + """ > + > + def test_migration_with_tcp_localhost(self): > + self.migration_with_tcp_localhost() > + > + def test_migration_with_unix(self): > + self.migration_with_unix() > + > + def test_migration_with_exec(self): > + self.migration_with_exec() > -- > 2.35.3 thanks -- PMM
Peter Maydell <peter.maydell@linaro.org> writes: > On Tue, 28 Feb 2023 at 19:28, Fabiano Rosas <farosas@suse.de> wrote: >> >> The migration tests are currently broken for an aarch64 host because >> the tests pass no 'machine' and 'cpu' options on the QEMU command >> line. >> >> Add a separate class to each architecture so that we can specify >> 'machine' and 'cpu' options instead of relying on defaults. >> >> Add a skip decorator to keep the current behavior of only running >> migration tests when the qemu target matches the host architecture. > > I still don't understand this patch. Don't we run the > migration-test on all hosts already? David ? > We run on all hosts but for each host we only take the QEMU binary that matches the host architecture. So if you want to test aarch64 migration, you need an aarch64 host. If you run on an x86_64 host (without this patch): $ ../configure #all targets $ make check-avocado AVOCADO_TESTS=../tests/avocado/migration.py You'll see: (1/3) ... migration.py:Migration.test_migration_with_tcp_localhost: PASS (0.21 s) (2/3) ... migration.py:Migration.test_migration_with_unix: PASS (0.18 s) (3/3) ... migration.py:Migration.test_migration_with_exec: PASS (0.21 s) All three tests ran using qemu-system-x86_64. The issue I'm trying to solve is that when run on a aarch64 host, the test will fail because (being generic) it doesn't pass the '-machine virt' option and there is no architecture-specific information in it at all. If we need to pass '-machine virt' to the arm machine, then something needs to change to add architecture-specific knowledge into the test. My first version hardcoded the usual "if arch == foo". That was frowned upon, so this version creates a class for each architecture like other tests do (e.g. boot_linux.py). The downside of this is that we need to explicitly enumerate the host architectures on which we want the test to run. I chose a few of the obvious, but we might need to add more. The upside is that we could now enable the test to run with all the targets present in the build. If we remove the @skip decorators from this patch, we'd get (note the arch strings): (01/12) ... migration.py:Aarch64.test_migration_with_tcp_localhost: PASS (0.19 s) (02/12) ... migration.py:Aarch64.test_migration_with_unix: PASS (0.16 s) (03/12) ... migration.py:Aarch64.test_migration_with_exec: PASS (0.20 s) ^ (04/12) ... migration.py:X86_64.test_migration_with_tcp_localhost: PASS (0.21 s) (05/12) ... migration.py:X86_64.test_migration_with_unix: PASS (0.18 s) (06/12) ... migration.py:X86_64.test_migration_with_exec: PASS (0.21 s) ^ (07/12) ... migration.py:PPC64.test_migration_with_tcp_localhost: PASS (0.20 s) (08/12) ... migration.py:PPC64.test_migration_with_unix: PASS (0.17 s) (09/12) ... migration.py:PPC64.test_migration_with_exec: PASS (0.20 s) ^
On Fri, 3 Mar 2023 at 20:59, Fabiano Rosas <farosas@suse.de> wrote: > > Peter Maydell <peter.maydell@linaro.org> writes: > > > On Tue, 28 Feb 2023 at 19:28, Fabiano Rosas <farosas@suse.de> wrote: > >> > >> The migration tests are currently broken for an aarch64 host because > >> the tests pass no 'machine' and 'cpu' options on the QEMU command > >> line. > >> > >> Add a separate class to each architecture so that we can specify > >> 'machine' and 'cpu' options instead of relying on defaults. > >> > >> Add a skip decorator to keep the current behavior of only running > >> migration tests when the qemu target matches the host architecture. > > > > I still don't understand this patch. Don't we run the > > migration-test on all hosts already? David ? > > > > We run on all hosts but for each host we only take the QEMU binary that > matches the host architecture. So if you want to test aarch64 migration, > you need an aarch64 host. > > If you run on an x86_64 host (without this patch): > $ ../configure #all targets > $ make check-avocado AVOCADO_TESTS=../tests/avocado/migration.py > > You'll see: > > (1/3) ... migration.py:Migration.test_migration_with_tcp_localhost: PASS (0.21 s) > (2/3) ... migration.py:Migration.test_migration_with_unix: PASS (0.18 s) > (3/3) ... migration.py:Migration.test_migration_with_exec: PASS (0.21 s) > > All three tests ran using qemu-system-x86_64. > > The issue I'm trying to solve is that when run on a aarch64 host, the > test will fail because (being generic) it doesn't pass the '-machine > virt' option and there is no architecture-specific information in it at > all. But my point is that we already CI on aarch64 hosts, so what is happening there that means the test doesn't fail already ? -- PMM
* Peter Maydell (peter.maydell@linaro.org) wrote: > On Tue, 28 Feb 2023 at 19:28, Fabiano Rosas <farosas@suse.de> wrote: > > > > The migration tests are currently broken for an aarch64 host because > > the tests pass no 'machine' and 'cpu' options on the QEMU command > > line. > > > > Add a separate class to each architecture so that we can specify > > 'machine' and 'cpu' options instead of relying on defaults. > > > > Add a skip decorator to keep the current behavior of only running > > migration tests when the qemu target matches the host architecture. > > I still don't understand this patch. Don't we run the > migration-test on all hosts already? David ? I don't run the avocado tests. Dave > > Signed-off-by: Fabiano Rosas <farosas@suse.de> > > --- > > tests/avocado/migration.py | 83 +++++++++++++++++++++++++++++++++++--- > > 1 file changed, 78 insertions(+), 5 deletions(-) > > > > diff --git a/tests/avocado/migration.py b/tests/avocado/migration.py > > index 4b25680c50..8b2ec0e3c4 100644 > > --- a/tests/avocado/migration.py > > +++ b/tests/avocado/migration.py > > @@ -11,6 +11,8 @@ > > > > > > import tempfile > > +import os > > + > > from avocado_qemu import QemuSystemTest > > from avocado import skipUnless > > > > @@ -19,7 +21,7 @@ > > from avocado.utils.path import find_command > > > > > > -class Migration(QemuSystemTest): > > +class MigrationTest(QemuSystemTest): > > """ > > :avocado: tags=migration > > """ > > @@ -62,20 +64,91 @@ def _get_free_port(self): > > self.cancel('Failed to find a free port') > > return port > > > > - > > - def test_migration_with_tcp_localhost(self): > > + def migration_with_tcp_localhost(self): > > dest_uri = 'tcp:localhost:%u' % self._get_free_port() > > self.do_migrate(dest_uri) > > > > - def test_migration_with_unix(self): > > + def migration_with_unix(self): > > with tempfile.TemporaryDirectory(prefix='socket_') as socket_path: > > dest_uri = 'unix:%s/qemu-test.sock' % socket_path > > self.do_migrate(dest_uri) > > > > @skipUnless(find_command('nc', default=False), "'nc' command not found") > > - def test_migration_with_exec(self): > > + def migration_with_exec(self): > > """The test works for both netcat-traditional and netcat-openbsd packages.""" > > free_port = self._get_free_port() > > dest_uri = 'exec:nc -l localhost %u' % free_port > > src_uri = 'exec:nc localhost %u' % free_port > > self.do_migrate(dest_uri, src_uri) > > + > > + > > +@skipUnless('aarch64' in os.uname()[4], "host != target") > > +class Aarch64(MigrationTest): > > + """ > > + :avocado: tags=arch:aarch64 > > + :avocado: tags=machine:virt > > + :avocado: tags=cpu:max > > + """ > > + > > + def test_migration_with_tcp_localhost(self): > > + self.migration_with_tcp_localhost() > > + > > + def test_migration_with_unix(self): > > + self.migration_with_unix() > > + > > + def test_migration_with_exec(self): > > + self.migration_with_exec() > > + > > + > > +@skipUnless('x86_64' in os.uname()[4], "host != target") > > +class X86_64(MigrationTest): > > + """ > > + :avocado: tags=arch:x86_64 > > + :avocado: tags=machine:pc > > + :avocado: tags=cpu:qemu64 > > + """ > > + > > + def test_migration_with_tcp_localhost(self): > > + self.migration_with_tcp_localhost() > > + > > + def test_migration_with_unix(self): > > + self.migration_with_unix() > > + > > + def test_migration_with_exec(self): > > + self.migration_with_exec() > > + > > + > > +@skipUnless('ppc64le' in os.uname()[4], "host != target") > > +class PPC64(MigrationTest): > > + """ > > + :avocado: tags=arch:ppc64 > > + :avocado: tags=machine:pseries > > + :avocado: tags=cpu:power9_v2.0 > > + """ > > + > > + def test_migration_with_tcp_localhost(self): > > + self.migration_with_tcp_localhost() > > + > > + def test_migration_with_unix(self): > > + self.migration_with_unix() > > + > > + def test_migration_with_exec(self): > > + self.migration_with_exec() > > + > > + > > +@skipUnless('s390x' in os.uname()[4], "host != target") > > +class S390X(MigrationTest): > > + """ > > + :avocado: tags=arch:s390x > > + :avocado: tags=machine:s390-ccw-virtio > > + :avocado: tags=cpu:qemu > > + """ > > + > > + def test_migration_with_tcp_localhost(self): > > + self.migration_with_tcp_localhost() > > + > > + def test_migration_with_unix(self): > > + self.migration_with_unix() > > + > > + def test_migration_with_exec(self): > > + self.migration_with_exec() > > -- > > 2.35.3 > > thanks > -- PMM >
Peter Maydell <peter.maydell@linaro.org> writes: > On Fri, 3 Mar 2023 at 20:59, Fabiano Rosas <farosas@suse.de> wrote: >> >> Peter Maydell <peter.maydell@linaro.org> writes: >> >> > On Tue, 28 Feb 2023 at 19:28, Fabiano Rosas <farosas@suse.de> wrote: >> >> >> >> The migration tests are currently broken for an aarch64 host because >> >> the tests pass no 'machine' and 'cpu' options on the QEMU command >> >> line. >> >> >> >> Add a separate class to each architecture so that we can specify >> >> 'machine' and 'cpu' options instead of relying on defaults. >> >> >> >> Add a skip decorator to keep the current behavior of only running >> >> migration tests when the qemu target matches the host architecture. >> > >> > I still don't understand this patch. Don't we run the >> > migration-test on all hosts already? David ? >> > >> >> We run on all hosts but for each host we only take the QEMU binary that >> matches the host architecture. So if you want to test aarch64 migration, >> you need an aarch64 host. >> >> If you run on an x86_64 host (without this patch): >> $ ../configure #all targets >> $ make check-avocado AVOCADO_TESTS=../tests/avocado/migration.py >> >> You'll see: >> >> (1/3) ... migration.py:Migration.test_migration_with_tcp_localhost: PASS (0.21 s) >> (2/3) ... migration.py:Migration.test_migration_with_unix: PASS (0.18 s) >> (3/3) ... migration.py:Migration.test_migration_with_exec: PASS (0.21 s) >> >> All three tests ran using qemu-system-x86_64. >> >> The issue I'm trying to solve is that when run on a aarch64 host, the >> test will fail because (being generic) it doesn't pass the '-machine >> virt' option and there is no architecture-specific information in it at >> all. > > But my point is that we already CI on aarch64 hosts, so what is > happening there that means the test doesn't fail already ? > I don't see check-avocado on the custom runners job descriptions (.gitlab-ci.d/custom-runners/ubuntu-22.04-aarch64.yml). It seems we don't run avocado there at all. For the regular jobs, there's the avocado-cfi-aarch64 job which depends on the build-cfi-aarch64 job that is being skipped at the moment. But that wouldn't catch this bug because it runs on an x86 host and this particular test gets skipped.
diff --git a/tests/avocado/migration.py b/tests/avocado/migration.py index 4b25680c50..8b2ec0e3c4 100644 --- a/tests/avocado/migration.py +++ b/tests/avocado/migration.py @@ -11,6 +11,8 @@ import tempfile +import os + from avocado_qemu import QemuSystemTest from avocado import skipUnless @@ -19,7 +21,7 @@ from avocado.utils.path import find_command -class Migration(QemuSystemTest): +class MigrationTest(QemuSystemTest): """ :avocado: tags=migration """ @@ -62,20 +64,91 @@ def _get_free_port(self): self.cancel('Failed to find a free port') return port - - def test_migration_with_tcp_localhost(self): + def migration_with_tcp_localhost(self): dest_uri = 'tcp:localhost:%u' % self._get_free_port() self.do_migrate(dest_uri) - def test_migration_with_unix(self): + def migration_with_unix(self): with tempfile.TemporaryDirectory(prefix='socket_') as socket_path: dest_uri = 'unix:%s/qemu-test.sock' % socket_path self.do_migrate(dest_uri) @skipUnless(find_command('nc', default=False), "'nc' command not found") - def test_migration_with_exec(self): + def migration_with_exec(self): """The test works for both netcat-traditional and netcat-openbsd packages.""" free_port = self._get_free_port() dest_uri = 'exec:nc -l localhost %u' % free_port src_uri = 'exec:nc localhost %u' % free_port self.do_migrate(dest_uri, src_uri) + + +@skipUnless('aarch64' in os.uname()[4], "host != target") +class Aarch64(MigrationTest): + """ + :avocado: tags=arch:aarch64 + :avocado: tags=machine:virt + :avocado: tags=cpu:max + """ + + def test_migration_with_tcp_localhost(self): + self.migration_with_tcp_localhost() + + def test_migration_with_unix(self): + self.migration_with_unix() + + def test_migration_with_exec(self): + self.migration_with_exec() + + +@skipUnless('x86_64' in os.uname()[4], "host != target") +class X86_64(MigrationTest): + """ + :avocado: tags=arch:x86_64 + :avocado: tags=machine:pc + :avocado: tags=cpu:qemu64 + """ + + def test_migration_with_tcp_localhost(self): + self.migration_with_tcp_localhost() + + def test_migration_with_unix(self): + self.migration_with_unix() + + def test_migration_with_exec(self): + self.migration_with_exec() + + +@skipUnless('ppc64le' in os.uname()[4], "host != target") +class PPC64(MigrationTest): + """ + :avocado: tags=arch:ppc64 + :avocado: tags=machine:pseries + :avocado: tags=cpu:power9_v2.0 + """ + + def test_migration_with_tcp_localhost(self): + self.migration_with_tcp_localhost() + + def test_migration_with_unix(self): + self.migration_with_unix() + + def test_migration_with_exec(self): + self.migration_with_exec() + + +@skipUnless('s390x' in os.uname()[4], "host != target") +class S390X(MigrationTest): + """ + :avocado: tags=arch:s390x + :avocado: tags=machine:s390-ccw-virtio + :avocado: tags=cpu:qemu + """ + + def test_migration_with_tcp_localhost(self): + self.migration_with_tcp_localhost() + + def test_migration_with_unix(self): + self.migration_with_unix() + + def test_migration_with_exec(self): + self.migration_with_exec()
The migration tests are currently broken for an aarch64 host because the tests pass no 'machine' and 'cpu' options on the QEMU command line. Add a separate class to each architecture so that we can specify 'machine' and 'cpu' options instead of relying on defaults. Add a skip decorator to keep the current behavior of only running migration tests when the qemu target matches the host architecture. Signed-off-by: Fabiano Rosas <farosas@suse.de> --- tests/avocado/migration.py | 83 +++++++++++++++++++++++++++++++++++--- 1 file changed, 78 insertions(+), 5 deletions(-)