tests/qtest/vhost-user-blk-test: don't abort all qtests on missing envar

[PATCH] tests/qtest/vhost-user-blk-test: don't abort all qtests on missing envar

Posted by Christian Schoenebeck 3 years, 2 months ago

This test requires environment variable QTEST_QEMU_STORAGE_DAEMON_BINARY
to be defined for running. If not, it would immediately abort all qtests
and prevent other, unrelated tests from running.

To fix that, just skip vhost-user-blk-test instead and log a message
about missing environment variable.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---

 I also tried g_test_skip(errmsg) from the setup handlers instead, but it
 always caused the tests to abort with an error:
 
 ../tests/qtest/libqtest.c:179: kill_qemu() tried to terminate QEMU process
 but encountered exit status 1 (expected 0)
 
 I haven't further investigated.

 tests/qtest/vhost-user-blk-test.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tests/qtest/vhost-user-blk-test.c b/tests/qtest/vhost-user-blk-test.c
index 07a4c2d500..dc37f5af4d 100644
--- a/tests/qtest/vhost-user-blk-test.c
+++ b/tests/qtest/vhost-user-blk-test.c
@@ -983,6 +983,12 @@ static void register_vhost_user_blk_test(void)
         .before = vhost_user_blk_test_setup,
     };
 
+    if (!getenv("QTEST_QEMU_STORAGE_DAEMON_BINARY")) {
+        g_test_message("QTEST_QEMU_STORAGE_DAEMON_BINARY not defined, "
+                       "skipping vhost-user-blk-test");
+        return;
+    }
+
     /*
      * tests for vhost-user-blk and vhost-user-blk-pci
      * The tests are borrowed from tests/virtio-blk-test.c. But some tests
-- 
2.30.2

Re: [PATCH] tests/qtest/vhost-user-blk-test: don't abort all qtests on missing envar

Posted by Thomas Huth 3 years, 1 month ago

On 25/11/2022 16.58, Christian Schoenebeck wrote:
> This test requires environment variable QTEST_QEMU_STORAGE_DAEMON_BINARY
> to be defined for running. If not, it would immediately abort all qtests
> and prevent other, unrelated tests from running.
> 
> To fix that, just skip vhost-user-blk-test instead and log a message
> about missing environment variable.
> 
> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> ---
> 
>   I also tried g_test_skip(errmsg) from the setup handlers instead, but it
>   always caused the tests to abort with an error:
>   
>   ../tests/qtest/libqtest.c:179: kill_qemu() tried to terminate QEMU process
>   but encountered exit status 1 (expected 0)
>   
>   I haven't further investigated.
> 
>   tests/qtest/vhost-user-blk-test.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/tests/qtest/vhost-user-blk-test.c b/tests/qtest/vhost-user-blk-test.c
> index 07a4c2d500..dc37f5af4d 100644
> --- a/tests/qtest/vhost-user-blk-test.c
> +++ b/tests/qtest/vhost-user-blk-test.c
> @@ -983,6 +983,12 @@ static void register_vhost_user_blk_test(void)
>           .before = vhost_user_blk_test_setup,
>       };
>   
> +    if (!getenv("QTEST_QEMU_STORAGE_DAEMON_BINARY")) {
> +        g_test_message("QTEST_QEMU_STORAGE_DAEMON_BINARY not defined, "
> +                       "skipping vhost-user-blk-test");
> +        return;
> +    }
> +
>       /*
>        * tests for vhost-user-blk and vhost-user-blk-pci
>        * The tests are borrowed from tests/virtio-blk-test.c. But some tests

Thanks, queued to my testing-next branch now:

  https://gitlab.com/thuth/qemu/-/commits/testing-next/

  Thomas

Re: [PATCH] tests/qtest/vhost-user-blk-test: don't abort all qtests on missing envar

Posted by Thomas Huth 3 years, 2 months ago

On 25/11/2022 16.58, Christian Schoenebeck wrote:
> This test requires environment variable QTEST_QEMU_STORAGE_DAEMON_BINARY
> to be defined for running. If not, it would immediately abort all qtests
> and prevent other, unrelated tests from running.
> 
> To fix that, just skip vhost-user-blk-test instead and log a message
> about missing environment variable.
> 
> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> ---
> 
>   I also tried g_test_skip(errmsg) from the setup handlers instead, but it
>   always caused the tests to abort with an error:
>   
>   ../tests/qtest/libqtest.c:179: kill_qemu() tried to terminate QEMU process
>   but encountered exit status 1 (expected 0)
>   
>   I haven't further investigated.
> 
>   tests/qtest/vhost-user-blk-test.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/tests/qtest/vhost-user-blk-test.c b/tests/qtest/vhost-user-blk-test.c
> index 07a4c2d500..dc37f5af4d 100644
> --- a/tests/qtest/vhost-user-blk-test.c
> +++ b/tests/qtest/vhost-user-blk-test.c
> @@ -983,6 +983,12 @@ static void register_vhost_user_blk_test(void)
>           .before = vhost_user_blk_test_setup,
>       };
>   
> +    if (!getenv("QTEST_QEMU_STORAGE_DAEMON_BINARY")) {
> +        g_test_message("QTEST_QEMU_STORAGE_DAEMON_BINARY not defined, "
> +                       "skipping vhost-user-blk-test");
> +        return;
> +    }

Could we use g_test_skip() here?

  Thomas

Re: [PATCH] tests/qtest/vhost-user-blk-test: don't abort all qtests on missing envar

Posted by Thomas Huth 3 years, 2 months ago

On 01/12/2022 12.03, Thomas Huth wrote:
> On 25/11/2022 16.58, Christian Schoenebeck wrote:
>> This test requires environment variable QTEST_QEMU_STORAGE_DAEMON_BINARY
>> to be defined for running. If not, it would immediately abort all qtests
>> and prevent other, unrelated tests from running.
>>
>> To fix that, just skip vhost-user-blk-test instead and log a message
>> about missing environment variable.
>>
>> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
>> ---
>>
>>   I also tried g_test_skip(errmsg) from the setup handlers instead, but it
>>   always caused the tests to abort with an error:
>>   ../tests/qtest/libqtest.c:179: kill_qemu() tried to terminate QEMU process
>>   but encountered exit status 1 (expected 0)
>>   I haven't further investigated.
>>
>>   tests/qtest/vhost-user-blk-test.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/tests/qtest/vhost-user-blk-test.c 
>> b/tests/qtest/vhost-user-blk-test.c
>> index 07a4c2d500..dc37f5af4d 100644
>> --- a/tests/qtest/vhost-user-blk-test.c
>> +++ b/tests/qtest/vhost-user-blk-test.c
>> @@ -983,6 +983,12 @@ static void register_vhost_user_blk_test(void)
>>           .before = vhost_user_blk_test_setup,
>>       };
>> +    if (!getenv("QTEST_QEMU_STORAGE_DAEMON_BINARY")) {
>> +        g_test_message("QTEST_QEMU_STORAGE_DAEMON_BINARY not defined, "
>> +                       "skipping vhost-user-blk-test");
>> +        return;
>> +    }
> 
> Could we use g_test_skip() here?

Maybe I should also read the lines between "---" and "diff", sorry!

Would be interesting to know why it does not work, though, anyway, for this 
patch here:

Reviewed-by: Thomas Huth <thuth@redhat.com>