Unfortunately, this test had not been added to meson.build, so we did
not notice a regression: Looking for 'Kernel panic - not syncing: VFS:'
as the indication for the final boot state of the kernel was a bad
idea since 'Kernel panic - not syncing' is the default failure
message of the LinuxKernelTest class, and since we're now reading
the console input byte by byte instead of linewise (see commit
cdad03b74f75), the failure now triggers before we fully read the
success string. Let's fix this by simply looking for the previous
line in the console output instead.

Also, replace the call to cancel() - this was only available in the
Avocado framework. In the functional framework, we must use skipTest()
instead.

Fixes: 3abc545e66 ("tests/functional: Convert the tcg_plugins test")
Fixes: cdad03b74f ("tests/functional: rewrite console handling to be bytewise")
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/functional/meson.build                 | 1 +
 tests/functional/test_aarch64_tcg_plugins.py | 6 +++---
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/tests/functional/meson.build b/tests/functional/meson.build
index 5457331643..e0a276f349 100644
--- a/tests/functional/meson.build
+++ b/tests/functional/meson.build
@@ -72,6 +72,7 @@ tests_aarch64_system_thorough = [
   'aarch64_sbsaref',
   'aarch64_sbsaref_alpine',
   'aarch64_sbsaref_freebsd',
+  'aarch64_tcg_plugins',
   'aarch64_tuxrun',
   'aarch64_virt',
   'aarch64_xlnx_versal',
diff --git a/tests/functional/test_aarch64_tcg_plugins.py b/tests/functional/test_aarch64_tcg_plugins.py
index 01660eb090..357eb48477 100755
--- a/tests/functional/test_aarch64_tcg_plugins.py
+++ b/tests/functional/test_aarch64_tcg_plugins.py
@@ -46,7 +46,7 @@ def run_vm(self, kernel_path, kernel_command_line,
         except:
             # TODO: probably fails because plugins not enabled but we
             # can't currently probe for the feature.
-            self.cancel("TCG Plugins not enabled?")
+            self.skipTest("TCG Plugins not enabled?")
 
         self.wait_for_console_pattern(console_pattern, vm)
         # ensure logs are flushed
@@ -65,7 +65,7 @@ def test_aarch64_virt_insn(self):
         kernel_path = self.ASSET_KERNEL.fetch()
         kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
                                'console=ttyAMA0')
-        console_pattern = 'Kernel panic - not syncing: VFS:'
+        console_pattern = 'Please append a correct "root=" boot option'
 
         plugin_log = tempfile.NamedTemporaryFile(mode="r+t", prefix="plugin",
                                                  suffix=".log")
@@ -91,7 +91,7 @@ def test_aarch64_virt_insn_icount(self):
         kernel_path = self.ASSET_KERNEL.fetch()
         kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
                                'console=ttyAMA0')
-        console_pattern = 'Kernel panic - not syncing: VFS:'
+        console_pattern = 'Please append a correct "root=" boot option'
 
         plugin_log = tempfile.NamedTemporaryFile(mode="r+t", prefix="plugin",
                                                  suffix=".log")
-- 
2.48.1

Thomas Huth <thuth@redhat.com> writes:

> Unfortunately, this test had not been added to meson.build, so we did
> not notice a regression: Looking for 'Kernel panic - not syncing: VFS:'
> as the indication for the final boot state of the kernel was a bad
> idea since 'Kernel panic - not syncing' is the default failure
> message of the LinuxKernelTest class, and since we're now reading
> the console input byte by byte instead of linewise (see commit
> cdad03b74f75), the failure now triggers before we fully read the
> success string. Let's fix this by simply looking for the previous
> line in the console output instead.
>
> Also, replace the call to cancel() - this was only available in the
> Avocado framework. In the functional framework, we must use skipTest()
> instead.
>
> Fixes: 3abc545e66 ("tests/functional: Convert the tcg_plugins test")
> Fixes: cdad03b74f ("tests/functional: rewrite console handling to be bytewise")
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  tests/functional/meson.build                 | 1 +
>  tests/functional/test_aarch64_tcg_plugins.py | 6 +++---
>  2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/tests/functional/meson.build b/tests/functional/meson.build
> index 5457331643..e0a276f349 100644
> --- a/tests/functional/meson.build
> +++ b/tests/functional/meson.build
> @@ -72,6 +72,7 @@ tests_aarch64_system_thorough = [
>    'aarch64_sbsaref',
>    'aarch64_sbsaref_alpine',
>    'aarch64_sbsaref_freebsd',
> +  'aarch64_tcg_plugins',

The test only takes a few seconds to run so maybe it can go in the basic tests?

>    'aarch64_tuxrun',
>    'aarch64_virt',
>    'aarch64_xlnx_versal',
> diff --git a/tests/functional/test_aarch64_tcg_plugins.py b/tests/functional/test_aarch64_tcg_plugins.py
> index 01660eb090..357eb48477 100755
> --- a/tests/functional/test_aarch64_tcg_plugins.py
> +++ b/tests/functional/test_aarch64_tcg_plugins.py
> @@ -46,7 +46,7 @@ def run_vm(self, kernel_path, kernel_command_line,
>          except:
>              # TODO: probably fails because plugins not enabled but we
>              # can't currently probe for the feature.
> -            self.cancel("TCG Plugins not enabled?")
> +            self.skipTest("TCG Plugins not enabled?")

We can be more explicit now, something like:

        try:
            vm.launch()
        except VMLaunchFailure as excp:
            if "plugin interface not enabled in this build" in excp.output:
                self.skipTest("TCG plugins not enabled")
            else:
                self.log.info(f"unhandled launch failure: {excp.output}")
                raise excp


>  
>          self.wait_for_console_pattern(console_pattern, vm)
>          # ensure logs are flushed
> @@ -65,7 +65,7 @@ def test_aarch64_virt_insn(self):
>          kernel_path = self.ASSET_KERNEL.fetch()
>          kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
>                                 'console=ttyAMA0')
> -        console_pattern = 'Kernel panic - not syncing: VFS:'
> +        console_pattern = 'Please append a correct "root=" boot option'
>  
>          plugin_log = tempfile.NamedTemporaryFile(mode="r+t", prefix="plugin",
>                                                   suffix=".log")
> @@ -91,7 +91,7 @@ def test_aarch64_virt_insn_icount(self):
>          kernel_path = self.ASSET_KERNEL.fetch()
>          kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
>                                 'console=ttyAMA0')
> -        console_pattern = 'Kernel panic - not syncing: VFS:'
> +        console_pattern = 'Please append a correct "root=" boot option'
>  
>          plugin_log = tempfile.NamedTemporaryFile(mode="r+t", prefix="plugin",
>                                                   suffix=".log")

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

On 22/01/2025 18.33, Alex Bennée wrote:
> Thomas Huth <thuth@redhat.com> writes:
> 
>> Unfortunately, this test had not been added to meson.build, so we did
>> not notice a regression: Looking for 'Kernel panic - not syncing: VFS:'
>> as the indication for the final boot state of the kernel was a bad
>> idea since 'Kernel panic - not syncing' is the default failure
>> message of the LinuxKernelTest class, and since we're now reading
>> the console input byte by byte instead of linewise (see commit
>> cdad03b74f75), the failure now triggers before we fully read the
>> success string. Let's fix this by simply looking for the previous
>> line in the console output instead.
>>
>> Also, replace the call to cancel() - this was only available in the
>> Avocado framework. In the functional framework, we must use skipTest()
>> instead.
>>
>> Fixes: 3abc545e66 ("tests/functional: Convert the tcg_plugins test")
>> Fixes: cdad03b74f ("tests/functional: rewrite console handling to be bytewise")
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   tests/functional/meson.build                 | 1 +
>>   tests/functional/test_aarch64_tcg_plugins.py | 6 +++---
>>   2 files changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/tests/functional/meson.build b/tests/functional/meson.build
>> index 5457331643..e0a276f349 100644
>> --- a/tests/functional/meson.build
>> +++ b/tests/functional/meson.build
>> @@ -72,6 +72,7 @@ tests_aarch64_system_thorough = [
>>     'aarch64_sbsaref',
>>     'aarch64_sbsaref_alpine',
>>     'aarch64_sbsaref_freebsd',
>> +  'aarch64_tcg_plugins',
> 
> The test only takes a few seconds to run so maybe it can go in the basic tests?

Unfortunately no, the quick category is reserved for the tests that don't 
need to download any assets, and this one here is downloading some stuff.

>>     'aarch64_tuxrun',
>>     'aarch64_virt',
>>     'aarch64_xlnx_versal',
>> diff --git a/tests/functional/test_aarch64_tcg_plugins.py b/tests/functional/test_aarch64_tcg_plugins.py
>> index 01660eb090..357eb48477 100755
>> --- a/tests/functional/test_aarch64_tcg_plugins.py
>> +++ b/tests/functional/test_aarch64_tcg_plugins.py
>> @@ -46,7 +46,7 @@ def run_vm(self, kernel_path, kernel_command_line,
>>           except:
>>               # TODO: probably fails because plugins not enabled but we
>>               # can't currently probe for the feature.
>> -            self.cancel("TCG Plugins not enabled?")
>> +            self.skipTest("TCG Plugins not enabled?")
> 
> We can be more explicit now, something like:
> 
>          try:
>              vm.launch()
>          except VMLaunchFailure as excp:
>              if "plugin interface not enabled in this build" in excp.output:
>                  self.skipTest("TCG plugins not enabled")
>              else:
>                  self.log.info(f"unhandled launch failure: {excp.output}")
>                  raise excp

Ok, good idea, I can respin a v2 with that included.

  Thomas