[PATCH for 6.1 0/6] virtio-gpu acceptance test: use current device name

[PATCH for 6.1 0/6] virtio-gpu acceptance test: use current device name

[Cleber Rosa]

The virtio-gpu acceptance tests are currently broken because of a
mismatch in the device option and name (see patch 5).

The exact failure is:

  qemu-system-x86_64: -device virtio-vga,virgl=on: Property 'virtio-vga.virgl' not found

While at it, I've included a few cleanups and improvements to the very
same test file.

A pipeline can be seen here (applied top of my python-next):

   https://gitlab.com/cleber.gnu/qemu/-/pipelines/337000139

I've also applied it on top of current master (4aa2454d9) and verified
the fix.

If all patches are considered too much for the soft freeze, I can
re-send patch 5 by itself.

Cleber Rosa (6):
  tests/acceptance/virtio-gpu.py: use require_accelerator()
  tests/acceptance/virtio-gpu.py: combine x86_64 arch tags
  tests/acceptance/virtio-gpu.py: combine CPU tags
  tests/acceptance/virtio-gpu.py: combine kernel command line
  tests/acceptance/virtio-gpu.py: use virtio-vga-gl
  tests/acceptance/virtio-gpu.py: provide kernel and initrd hashes

 tests/acceptance/virtio-gpu.py | 42 ++++++++++++----------------------
 1 file changed, 15 insertions(+), 27 deletions(-)

-- 
2.31.1

[PATCH 1/6] tests/acceptance/virtio-gpu.py: use require_accelerator()

[Cleber Rosa]

Since efe30d501 there's a shorthand for requiring specific
accelerators, and canceling the test if it's not available.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/acceptance/virtio-gpu.py | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/tests/acceptance/virtio-gpu.py b/tests/acceptance/virtio-gpu.py
index 589332c1b7..42602a240a 100644
--- a/tests/acceptance/virtio-gpu.py
+++ b/tests/acceptance/virtio-gpu.py
@@ -17,10 +17,6 @@
 import subprocess
 
 
-ACCEL_NOT_AVAILABLE_FMT = "%s accelerator does not seem to be available"
-KVM_NOT_AVAILABLE = ACCEL_NOT_AVAILABLE_FMT % "KVM"
-
-
 def pick_default_vug_bin():
     relative_path = "./contrib/vhost-user-gpu/vhost-user-gpu"
     if is_readable_executable_file(relative_path):
@@ -66,8 +62,7 @@ def test_virtio_vga_virgl(self):
             self.KERNEL_COMMON_COMMAND_LINE + "console=ttyS0 rdinit=/bin/bash"
         )
         # FIXME: should check presence of virtio, virgl etc
-        if not kvm_available(self.arch, self.qemu_bin):
-            self.cancel(KVM_NOT_AVAILABLE)
+        self.require_accelerator('kvm')
 
         kernel_path = self.fetch_asset(self.KERNEL_URL)
         initrd_path = self.fetch_asset(self.INITRD_URL)
@@ -107,8 +102,7 @@ def test_vhost_user_vga_virgl(self):
             self.KERNEL_COMMON_COMMAND_LINE + "console=ttyS0 rdinit=/bin/bash"
         )
         # FIXME: should check presence of vhost-user-gpu, virgl, memfd etc
-        if not kvm_available(self.arch, self.qemu_bin):
-            self.cancel(KVM_NOT_AVAILABLE)
+        self.require_accelerator('kvm')
 
         vug = pick_default_vug_bin()
         if not vug:
-- 
2.31.1

Re: [PATCH 1/6] tests/acceptance/virtio-gpu.py: use require_accelerator()

[Willian Rampazzo]

On Wed, Jul 14, 2021 at 2:41 PM Cleber Rosa <crosa@redhat.com> wrote:
>
> Since efe30d501 there's a shorthand for requiring specific
> accelerators, and canceling the test if it's not available.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/acceptance/virtio-gpu.py | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
>

Reviewed-by: Willian Rampazzo <willianr@redhat.com>

[PATCH 2/6] tests/acceptance/virtio-gpu.py: combine x86_64 arch tags

[Cleber Rosa]

The test class in question is x86_64 specific, so it's possible to set
the tags at the class level.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/acceptance/virtio-gpu.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tests/acceptance/virtio-gpu.py b/tests/acceptance/virtio-gpu.py
index 42602a240a..729b99b2e5 100644
--- a/tests/acceptance/virtio-gpu.py
+++ b/tests/acceptance/virtio-gpu.py
@@ -30,6 +30,7 @@ def pick_default_vug_bin():
 class VirtioGPUx86(Test):
     """
     :avocado: tags=virtio-gpu
+    :avocado: tags=arch:x86_64
     """
 
     KERNEL_COMMON_COMMAND_LINE = "printk.time=0 "
@@ -54,7 +55,6 @@ def wait_for_console_pattern(self, success_message, vm=None):
 
     def test_virtio_vga_virgl(self):
         """
-        :avocado: tags=arch:x86_64
         :avocado: tags=device:virtio-vga
         :avocado: tags=cpu:host
         """
@@ -94,7 +94,6 @@ def test_virtio_vga_virgl(self):
 
     def test_vhost_user_vga_virgl(self):
         """
-        :avocado: tags=arch:x86_64
         :avocado: tags=device:vhost-user-vga
         :avocado: tags=cpu:host
         """
-- 
2.31.1

Re: [PATCH 2/6] tests/acceptance/virtio-gpu.py: combine x86_64 arch tags

[Willian Rampazzo]

On Wed, Jul 14, 2021 at 2:41 PM Cleber Rosa <crosa@redhat.com> wrote:
>
> The test class in question is x86_64 specific, so it's possible to set
> the tags at the class level.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/acceptance/virtio-gpu.py | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>

Reviewed-by: Willian Rampazzo <willianr@redhat.com>

[PATCH 3/6] tests/acceptance/virtio-gpu.py: combine CPU tags

[Cleber Rosa]

Like previously done with the arch tags, all tests use the same CPU
value so it's possible to combine them at the class level.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/acceptance/virtio-gpu.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tests/acceptance/virtio-gpu.py b/tests/acceptance/virtio-gpu.py
index 729b99b2e5..20a59fabf3 100644
--- a/tests/acceptance/virtio-gpu.py
+++ b/tests/acceptance/virtio-gpu.py
@@ -31,6 +31,7 @@ class VirtioGPUx86(Test):
     """
     :avocado: tags=virtio-gpu
     :avocado: tags=arch:x86_64
+    :avocado: tags=cpu:host
     """
 
     KERNEL_COMMON_COMMAND_LINE = "printk.time=0 "
@@ -56,7 +57,6 @@ def wait_for_console_pattern(self, success_message, vm=None):
     def test_virtio_vga_virgl(self):
         """
         :avocado: tags=device:virtio-vga
-        :avocado: tags=cpu:host
         """
         kernel_command_line = (
             self.KERNEL_COMMON_COMMAND_LINE + "console=ttyS0 rdinit=/bin/bash"
@@ -95,7 +95,6 @@ def test_virtio_vga_virgl(self):
     def test_vhost_user_vga_virgl(self):
         """
         :avocado: tags=device:vhost-user-vga
-        :avocado: tags=cpu:host
         """
         kernel_command_line = (
             self.KERNEL_COMMON_COMMAND_LINE + "console=ttyS0 rdinit=/bin/bash"
-- 
2.31.1

Re: [PATCH 3/6] tests/acceptance/virtio-gpu.py: combine CPU tags

[Willian Rampazzo]

On Wed, Jul 14, 2021 at 2:41 PM Cleber Rosa <crosa@redhat.com> wrote:
>
> Like previously done with the arch tags, all tests use the same CPU
> value so it's possible to combine them at the class level.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/acceptance/virtio-gpu.py | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>

Reviewed-by: Willian Rampazzo <willianr@redhat.com>

[PATCH 4/6] tests/acceptance/virtio-gpu.py: combine kernel command line

[Cleber Rosa]

Both tests use the same kernel command line arguments, so there's no
need to have a common and then an additional set of arguments.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/acceptance/virtio-gpu.py | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/tests/acceptance/virtio-gpu.py b/tests/acceptance/virtio-gpu.py
index 20a59fabf3..fbde278705 100644
--- a/tests/acceptance/virtio-gpu.py
+++ b/tests/acceptance/virtio-gpu.py
@@ -34,7 +34,7 @@ class VirtioGPUx86(Test):
     :avocado: tags=cpu:host
     """
 
-    KERNEL_COMMON_COMMAND_LINE = "printk.time=0 "
+    KERNEL_COMMAND_LINE = "printk.time=0 console=ttyS0 rdinit=/bin/bash"
     KERNEL_URL = (
         "https://archives.fedoraproject.org/pub/fedora"
         "/linux/releases/33/Everything/x86_64/os/images"
@@ -58,9 +58,6 @@ def test_virtio_vga_virgl(self):
         """
         :avocado: tags=device:virtio-vga
         """
-        kernel_command_line = (
-            self.KERNEL_COMMON_COMMAND_LINE + "console=ttyS0 rdinit=/bin/bash"
-        )
         # FIXME: should check presence of virtio, virgl etc
         self.require_accelerator('kvm')
 
@@ -78,7 +75,7 @@ def test_virtio_vga_virgl(self):
             "-initrd",
             initrd_path,
             "-append",
-            kernel_command_line,
+            self.KERNEL_COMMAND_LINE,
         )
         try:
             self.vm.launch()
@@ -96,9 +93,6 @@ def test_vhost_user_vga_virgl(self):
         """
         :avocado: tags=device:vhost-user-vga
         """
-        kernel_command_line = (
-            self.KERNEL_COMMON_COMMAND_LINE + "console=ttyS0 rdinit=/bin/bash"
-        )
         # FIXME: should check presence of vhost-user-gpu, virgl, memfd etc
         self.require_accelerator('kvm')
 
@@ -145,7 +139,7 @@ def test_vhost_user_vga_virgl(self):
             "-initrd",
             initrd_path,
             "-append",
-            kernel_command_line,
+            self.KERNEL_COMMAND_LINE,
         )
         self.vm.launch()
         self.wait_for_console_pattern("as init process")
-- 
2.31.1

Re: [PATCH 4/6] tests/acceptance/virtio-gpu.py: combine kernel command line

[Willian Rampazzo]

On Wed, Jul 14, 2021 at 2:41 PM Cleber Rosa <crosa@redhat.com> wrote:
>
> Both tests use the same kernel command line arguments, so there's no
> need to have a common and then an additional set of arguments.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/acceptance/virtio-gpu.py | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
>

Reviewed-by: Willian Rampazzo <willianr@redhat.com>

[PATCH 5/6] tests/acceptance/virtio-gpu.py: use virtio-vga-gl

[Cleber Rosa]

Since 49afbca3b, the use of an optional virgl renderer is not
available anymore, and since b36eb8860f, the way to choose a GL based
rendered is to use the "virtio-vga-gl" device.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/acceptance/virtio-gpu.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/acceptance/virtio-gpu.py b/tests/acceptance/virtio-gpu.py
index fbde278705..0f84affe82 100644
--- a/tests/acceptance/virtio-gpu.py
+++ b/tests/acceptance/virtio-gpu.py
@@ -56,7 +56,7 @@ def wait_for_console_pattern(self, success_message, vm=None):
 
     def test_virtio_vga_virgl(self):
         """
-        :avocado: tags=device:virtio-vga
+        :avocado: tags=device:virtio-vga-gl
         """
         # FIXME: should check presence of virtio, virgl etc
         self.require_accelerator('kvm')
@@ -67,7 +67,7 @@ def test_virtio_vga_virgl(self):
         self.vm.set_console()
         self.vm.add_args("-m", "2G")
         self.vm.add_args("-machine", "pc,accel=kvm")
-        self.vm.add_args("-device", "virtio-vga,virgl=on")
+        self.vm.add_args("-device", "virtio-vga-gl")
         self.vm.add_args("-display", "egl-headless")
         self.vm.add_args(
             "-kernel",
-- 
2.31.1

Re: [PATCH 5/6] tests/acceptance/virtio-gpu.py: use virtio-vga-gl

[Willian Rampazzo]

On Wed, Jul 14, 2021 at 2:41 PM Cleber Rosa <crosa@redhat.com> wrote:
>
> Since 49afbca3b, the use of an optional virgl renderer is not
> available anymore, and since b36eb8860f, the way to choose a GL based
> rendered is to use the "virtio-vga-gl" device.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/acceptance/virtio-gpu.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>

Reviewed-by: Willian Rampazzo <willianr@redhat.com>

Re: [PATCH 5/6] tests/acceptance/virtio-gpu.py: use virtio-vga-gl

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 1524 bytes --]

On Wed, Jul 14, 2021 at 9:47 PM Cleber Rosa <crosa@redhat.com> wrote:

> Since 49afbca3b, the use of an optional virgl renderer is not
> available anymore, and since b36eb8860f, the way to choose a GL based
> rendered is to use the "virtio-vga-gl" device.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

---
>  tests/acceptance/virtio-gpu.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tests/acceptance/virtio-gpu.py
> b/tests/acceptance/virtio-gpu.py
> index fbde278705..0f84affe82 100644
> --- a/tests/acceptance/virtio-gpu.py
> +++ b/tests/acceptance/virtio-gpu.py
> @@ -56,7 +56,7 @@ def wait_for_console_pattern(self, success_message,
> vm=None):
>
>      def test_virtio_vga_virgl(self):
>          """
> -        :avocado: tags=device:virtio-vga
> +        :avocado: tags=device:virtio-vga-gl
>          """
>          # FIXME: should check presence of virtio, virgl etc
>          self.require_accelerator('kvm')
> @@ -67,7 +67,7 @@ def test_virtio_vga_virgl(self):
>          self.vm.set_console()
>          self.vm.add_args("-m", "2G")
>          self.vm.add_args("-machine", "pc,accel=kvm")
> -        self.vm.add_args("-device", "virtio-vga,virgl=on")
> +        self.vm.add_args("-device", "virtio-vga-gl")
>          self.vm.add_args("-display", "egl-headless")
>          self.vm.add_args(
>              "-kernel",
> --
> 2.31.1
>
>
>

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 2474 bytes --]

Re: [PATCH 5/6] tests/acceptance/virtio-gpu.py: use virtio-vga-gl

[Gerd Hoffmann]

On Wed, Jul 14, 2021 at 01:40:50PM -0400, Cleber Rosa wrote:
> Since 49afbca3b, the use of an optional virgl renderer is not
> available anymore, and since b36eb8860f, the way to choose a GL based
> rendered is to use the "virtio-vga-gl" device.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/acceptance/virtio-gpu.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/acceptance/virtio-gpu.py b/tests/acceptance/virtio-gpu.py
> index fbde278705..0f84affe82 100644
> --- a/tests/acceptance/virtio-gpu.py
> +++ b/tests/acceptance/virtio-gpu.py
> @@ -56,7 +56,7 @@ def wait_for_console_pattern(self, success_message, vm=None):
>  
>      def test_virtio_vga_virgl(self):
>          """
> -        :avocado: tags=device:virtio-vga
> +        :avocado: tags=device:virtio-vga-gl
>          """
>          # FIXME: should check presence of virtio, virgl etc
>          self.require_accelerator('kvm')
> @@ -67,7 +67,7 @@ def test_virtio_vga_virgl(self):
>          self.vm.set_console()
>          self.vm.add_args("-m", "2G")
>          self.vm.add_args("-machine", "pc,accel=kvm")
> -        self.vm.add_args("-device", "virtio-vga,virgl=on")
> +        self.vm.add_args("-device", "virtio-vga-gl")
>          self.vm.add_args("-display", "egl-headless")
>          self.vm.add_args(
>              "-kernel",

Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>

Side note: virgl=on isn't guaranteed to be present on old qemu versions,
it is only there on --enable-virglrenderer builds.  For upstream/master
this is fine, but in case you want your test cases work on older qemu
versions too you need to check that ...

take care,
  Gerd

[PATCH 6/6] tests/acceptance/virtio-gpu.py: provide kernel and initrd hashes

[Cleber Rosa]

By providing kernel and initrd hashes, the test guarantees the
integrity of the images used and avoids the warnings set by
fetch_asset() when hashes are lacking.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/acceptance/virtio-gpu.py | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tests/acceptance/virtio-gpu.py b/tests/acceptance/virtio-gpu.py
index 0f84affe82..4acc1e6d5f 100644
--- a/tests/acceptance/virtio-gpu.py
+++ b/tests/acceptance/virtio-gpu.py
@@ -40,11 +40,13 @@ class VirtioGPUx86(Test):
         "/linux/releases/33/Everything/x86_64/os/images"
         "/pxeboot/vmlinuz"
     )
+    KERNEL_HASH = '1433cfe3f2ffaa44de4ecfb57ec25dc2399cdecf'
     INITRD_URL = (
         "https://archives.fedoraproject.org/pub/fedora"
         "/linux/releases/33/Everything/x86_64/os/images"
         "/pxeboot/initrd.img"
     )
+    INITRD_HASH = 'c828d68a027b53e5220536585efe03412332c2d9'
 
     def wait_for_console_pattern(self, success_message, vm=None):
         wait_for_console_pattern(
@@ -61,8 +63,8 @@ def test_virtio_vga_virgl(self):
         # FIXME: should check presence of virtio, virgl etc
         self.require_accelerator('kvm')
 
-        kernel_path = self.fetch_asset(self.KERNEL_URL)
-        initrd_path = self.fetch_asset(self.INITRD_URL)
+        kernel_path = self.fetch_asset(self.KERNEL_URL, self.KERNEL_HASH)
+        initrd_path = self.fetch_asset(self.INITRD_URL, self.INITRD_HASH)
 
         self.vm.set_console()
         self.vm.add_args("-m", "2G")
@@ -100,8 +102,8 @@ def test_vhost_user_vga_virgl(self):
         if not vug:
             self.cancel("Could not find vhost-user-gpu")
 
-        kernel_path = self.fetch_asset(self.KERNEL_URL)
-        initrd_path = self.fetch_asset(self.INITRD_URL)
+        kernel_path = self.fetch_asset(self.KERNEL_URL, self.KERNEL_HASH)
+        initrd_path = self.fetch_asset(self.INITRD_URL, self.INITRD_HASH)
 
         # Create socketpair to connect proxy and remote processes
         qemu_sock, vug_sock = socket.socketpair(
-- 
2.31.1

Re: [PATCH 6/6] tests/acceptance/virtio-gpu.py: provide kernel and initrd hashes

[Willian Rampazzo]

On Wed, Jul 14, 2021 at 2:41 PM Cleber Rosa <crosa@redhat.com> wrote:
>
> By providing kernel and initrd hashes, the test guarantees the
> integrity of the images used and avoids the warnings set by
> fetch_asset() when hashes are lacking.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/acceptance/virtio-gpu.py | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>

Reviewed-by: Willian Rampazzo <willianr@redhat.com>