[Qemu-devel] [PATCH 09/10] vhost-user-test: small changes to init_hugepagefs

Paolo Bonzini posted 10 patches 6 years, 11 months ago
[Qemu-devel] [PATCH 09/10] vhost-user-test: small changes to init_hugepagefs
Posted by Paolo Bonzini 6 years, 11 months ago
After the conversion to qgraph, the equivalent of "main" will be in
a constructor and will run even if the tests are not being requested.
Therefore, it should not assert that init_hugepagefs succeeds and will
be called when creating the TestServer.  This patch changes the prototype
of init_hugepagefs, this way the next patch looks nicer.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/vhost-user-test.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
index 93d5157b13..a282fc57c8 100644
--- a/tests/vhost-user-test.c
+++ b/tests/vhost-user-test.c
@@ -461,13 +461,19 @@ static void chr_read(void *opaque, const uint8_t *buf, int size)
     g_mutex_unlock(&s->data_mutex);
 }
 
-static const char *init_hugepagefs(const char *path)
+static const char *init_hugepagefs(void)
 {
+    const char *path = getenv("QTEST_HUGETLBFS_PATH");
     struct statfs fs;
     int ret;
 
+    if (!path) {
+        return NULL;
+    }
+
     if (access(path, R_OK | W_OK | X_OK)) {
         g_test_message("access on path (%s): %s\n", path, strerror(errno));
+        abort();
         return NULL;
     }
 
@@ -477,11 +483,13 @@ static const char *init_hugepagefs(const char *path)
 
     if (ret != 0) {
         g_test_message("statfs on path (%s): %s\n", path, strerror(errno));
+        abort();
         return NULL;
     }
 
     if (fs.f_type != HUGETLBFS_MAGIC) {
         g_test_message("Warning: path not on HugeTLBFS: %s\n", path);
+        abort();
         return NULL;
     }
 
@@ -974,7 +982,6 @@ static void test_multiqueue(void)
 
 int main(int argc, char **argv)
 {
-    const char *hugefs;
     int ret;
     char template[] = "/tmp/vhost-test-XXXXXX";
 
@@ -989,13 +996,7 @@ int main(int argc, char **argv)
     }
     g_assert(tmpfs);
 
-    hugefs = getenv("QTEST_HUGETLBFS_PATH");
-    if (hugefs) {
-        root = init_hugepagefs(hugefs);
-        g_assert(root);
-    } else {
-        root = tmpfs;
-    }
+    root = init_hugepagefs() ? : tmpfs;
 
     if (qemu_memfd_check(0)) {
         qtest_add_data_func("/vhost-user/read-guest-mem/memfd",
-- 
2.19.1



Re: [Qemu-devel] [PATCH 09/10] vhost-user-test: small changes to init_hugepagefs
Posted by Philippe Mathieu-Daudé 6 years, 11 months ago
Hi Paolo,

On 15/11/18 15:31, Paolo Bonzini wrote:
> After the conversion to qgraph, the equivalent of "main" will be in
> a constructor and will run even if the tests are not being requested.
> Therefore, it should not assert that init_hugepagefs succeeds and will
> be called when creating the TestServer.  This patch changes the prototype
> of init_hugepagefs, this way the next patch looks nicer.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   tests/vhost-user-test.c | 19 ++++++++++---------
>   1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
> index 93d5157b13..a282fc57c8 100644
> --- a/tests/vhost-user-test.c
> +++ b/tests/vhost-user-test.c
> @@ -461,13 +461,19 @@ static void chr_read(void *opaque, const uint8_t *buf, int size)
>       g_mutex_unlock(&s->data_mutex);
>   }
>   
> -static const char *init_hugepagefs(const char *path)
> +static const char *init_hugepagefs(void)
>   {
> +    const char *path = getenv("QTEST_HUGETLBFS_PATH");
>       struct statfs fs;
>       int ret;
>   
> +    if (!path) {
> +        return NULL;
> +    }
> +
>       if (access(path, R_OK | W_OK | X_OK)) {
>           g_test_message("access on path (%s): %s\n", path, strerror(errno));
> +        abort();
>           return NULL;

Can we remove the 'return NULL's now?

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

>       }
>   
> @@ -477,11 +483,13 @@ static const char *init_hugepagefs(const char *path)
>   
>       if (ret != 0) {
>           g_test_message("statfs on path (%s): %s\n", path, strerror(errno));
> +        abort();
>           return NULL;
>       }
>   
>       if (fs.f_type != HUGETLBFS_MAGIC) {
>           g_test_message("Warning: path not on HugeTLBFS: %s\n", path);
> +        abort();
>           return NULL;
>       }
>   
> @@ -974,7 +982,6 @@ static void test_multiqueue(void)
>   
>   int main(int argc, char **argv)
>   {
> -    const char *hugefs;
>       int ret;
>       char template[] = "/tmp/vhost-test-XXXXXX";
>   
> @@ -989,13 +996,7 @@ int main(int argc, char **argv)
>       }
>       g_assert(tmpfs);
>   
> -    hugefs = getenv("QTEST_HUGETLBFS_PATH");
> -    if (hugefs) {
> -        root = init_hugepagefs(hugefs);
> -        g_assert(root);
> -    } else {
> -        root = tmpfs;
> -    }
> +    root = init_hugepagefs() ? : tmpfs;
>   
>       if (qemu_memfd_check(0)) {
>           qtest_add_data_func("/vhost-user/read-guest-mem/memfd",
> 

Re: [Qemu-devel] [PATCH 09/10] vhost-user-test: small changes to init_hugepagefs
Posted by Paolo Bonzini 6 years, 11 months ago
On 15/11/2018 15:55, Philippe Mathieu-Daudé wrote:
>>
>>       if (access(path, R_OK | W_OK | X_OK)) {
>>           g_test_message("access on path (%s): %s\n", path,
>> strerror(errno));
>> +        abort();
>>           return NULL;
> 
> Can we remove the 'return NULL's now?
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

I can remove them, but later on the aborts will become g_test_fail and
the test will continue without hugepagefs.

Paolo

Re: [Qemu-devel] [PATCH 09/10] vhost-user-test: small changes to init_hugepagefs
Posted by Philippe Mathieu-Daudé 6 years, 11 months ago
On 15/11/18 19:41, Paolo Bonzini wrote:
> On 15/11/2018 15:55, Philippe Mathieu-Daudé wrote:
>>>
>>>        if (access(path, R_OK | W_OK | X_OK)) {
>>>            g_test_message("access on path (%s): %s\n", path,
>>> strerror(errno));
>>> +        abort();
>>>            return NULL;
>>
>> Can we remove the 'return NULL's now?
>>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> I can remove them, but later on the aborts will become g_test_fail and
> the test will continue without hugepagefs.

OK, fine then!

Regards,

Phil.

Re: [Qemu-devel] [PATCH 09/10] vhost-user-test: small changes to init_hugepagefs
Posted by Paolo Bonzini 6 years, 11 months ago
On 15/11/2018 19:45, Philippe Mathieu-Daudé wrote:
> On 15/11/18 19:41, Paolo Bonzini wrote:
>> On 15/11/2018 15:55, Philippe Mathieu-Daudé wrote:
>>>>
>>>>        if (access(path, R_OK | W_OK | X_OK)) {
>>>>            g_test_message("access on path (%s): %s\n", path,
>>>> strerror(errno));
>>>> +        abort();
>>>>            return NULL;
>>>
>>> Can we remove the 'return NULL's now?
>>>
>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>
>> I can remove them, but later on the aborts will become g_test_fail and
>> the test will continue without hugepagefs.
> 
> OK, fine then!

Let's remove the returns here, and add them back together with
g_test_fail already in this series, in patch 10.

Paolo