[PATCH] tests/qtest/vhost-user-blk-test: Check whether qemu-storage-daemon is available

[PATCH] tests/qtest/vhost-user-blk-test: Check whether qemu-storage-daemon is available

[Thomas Huth]

The vhost-user-blk-test currently hangs if QTEST_QEMU_STORAGE_DAEMON_BINARY
points to a non-existing binary. Let's improve this situation by checking
for the availability of the binary first, so we can fail gracefully if
it is not accessible.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/qtest/vhost-user-blk-test.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tests/qtest/vhost-user-blk-test.c b/tests/qtest/vhost-user-blk-test.c
index 8796c74ca4..6f108a1b62 100644
--- a/tests/qtest/vhost-user-blk-test.c
+++ b/tests/qtest/vhost-user-blk-test.c
@@ -789,6 +789,14 @@ static const char *qtest_qemu_storage_daemon_binary(void)
         exit(0);
     }
 
+    /* If we've got a path to the binary, check whether we can access it */
+    if (strchr(qemu_storage_daemon_bin, '/') &&
+        access(qemu_storage_daemon_bin, X_OK) != 0) {
+        fprintf(stderr, "ERROR: '%s' is not accessible\n",
+                qemu_storage_daemon_bin);
+        exit(1);
+    }
+
     return qemu_storage_daemon_bin;
 }
 
-- 
2.27.0

Re: [PATCH] tests/qtest/vhost-user-blk-test: Check whether qemu-storage-daemon is available

[Alexander Bulekov]

On 210811 1159, Thomas Huth wrote:
> The vhost-user-blk-test currently hangs if QTEST_QEMU_STORAGE_DAEMON_BINARY
> points to a non-existing binary. Let's improve this situation by checking
> for the availability of the binary first, so we can fail gracefully if
> it is not accessible.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>

I manually removed ./storage-daemon/qemu-storage-daemon and re-ran
qos-test. The test errored-out without hanging.

Reviewed-by: Alexander Bulekov <alxndr@bu.edu>
Tested-by: Alexander Bulekov <alxndr@bu.edu>

Re: [PATCH] tests/qtest/vhost-user-blk-test: Check whether qemu-storage-daemon is available

[Peter Maydell]

On Wed, 11 Aug 2021 at 11:00, Thomas Huth <thuth@redhat.com> wrote:
>
> The vhost-user-blk-test currently hangs if QTEST_QEMU_STORAGE_DAEMON_BINARY
> points to a non-existing binary. Let's improve this situation by checking
> for the availability of the binary first, so we can fail gracefully if
> it is not accessible.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  tests/qtest/vhost-user-blk-test.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/tests/qtest/vhost-user-blk-test.c b/tests/qtest/vhost-user-blk-test.c
> index 8796c74ca4..6f108a1b62 100644
> --- a/tests/qtest/vhost-user-blk-test.c
> +++ b/tests/qtest/vhost-user-blk-test.c
> @@ -789,6 +789,14 @@ static const char *qtest_qemu_storage_daemon_binary(void)
>          exit(0);
>      }
>
> +    /* If we've got a path to the binary, check whether we can access it */
> +    if (strchr(qemu_storage_daemon_bin, '/') &&
> +        access(qemu_storage_daemon_bin, X_OK) != 0) {
> +        fprintf(stderr, "ERROR: '%s' is not accessible\n",
> +                qemu_storage_daemon_bin);
> +        exit(1);
> +    }

It makes sense not to bother starting the test if the binary isn't
even present, but why does the test hang? Shouldn't QEMU cleanly
exit rather than hanging if it turns out that it can't contact
the daemon ?

-- PMM

Re: [PATCH] tests/qtest/vhost-user-blk-test: Check whether qemu-storage-daemon is available

[Thomas Huth]

On 11/08/2021 13.08, Peter Maydell wrote:
> On Wed, 11 Aug 2021 at 11:00, Thomas Huth <thuth@redhat.com> wrote:
>>
>> The vhost-user-blk-test currently hangs if QTEST_QEMU_STORAGE_DAEMON_BINARY
>> points to a non-existing binary. Let's improve this situation by checking
>> for the availability of the binary first, so we can fail gracefully if
>> it is not accessible.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   tests/qtest/vhost-user-blk-test.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/tests/qtest/vhost-user-blk-test.c b/tests/qtest/vhost-user-blk-test.c
>> index 8796c74ca4..6f108a1b62 100644
>> --- a/tests/qtest/vhost-user-blk-test.c
>> +++ b/tests/qtest/vhost-user-blk-test.c
>> @@ -789,6 +789,14 @@ static const char *qtest_qemu_storage_daemon_binary(void)
>>           exit(0);
>>       }
>>
>> +    /* If we've got a path to the binary, check whether we can access it */
>> +    if (strchr(qemu_storage_daemon_bin, '/') &&
>> +        access(qemu_storage_daemon_bin, X_OK) != 0) {
>> +        fprintf(stderr, "ERROR: '%s' is not accessible\n",
>> +                qemu_storage_daemon_bin);
>> +        exit(1);
>> +    }
> 
> It makes sense not to bother starting the test if the binary isn't
> even present, but why does the test hang? Shouldn't QEMU cleanly
> exit rather than hanging if it turns out that it can't contact
> the daemon ?

Sorry for the late reply: I think this happens due to the way we run that 
qtest: The test program forks to run the storage daemon. If that daemon 
binary is not available, or exits prematurely, the original program does not 
notice and hangs. Maybe we should intercept the SIGCHLD signal for such cases?

  Thomas