[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 8/9] vhost-user-test: small changes to init_huge
From: |
Thomas Huth |
Subject: |
Re: [Qemu-devel] [PATCH 8/9] vhost-user-test: small changes to init_hugepagefs |
Date: |
Fri, 15 Feb 2019 09:27:44 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.5.0 |
On 14/02/2019 18.35, 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.
>
> Reviewed-by: Marc-André Lureau <address@hidden>
> Signed-off-by: Paolo Bonzini <address@hidden>
> Message-Id: <address@hidden>
> ---
> tests/vhost-user-test.c | 26 ++++++++++++++------------
> 1 file changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
> index 33030e0..516e31c 100644
> --- a/tests/vhost-user-test.c
> +++ b/tests/vhost-user-test.c
> @@ -465,14 +465,20 @@ static void chr_read(void *opaque, const uint8_t *buf,
> int size)
> g_mutex_unlock(&s->data_mutex);
> }
>
> -#ifdef CONFIG_LINUX
> -static const char *init_hugepagefs(const char *path)
> +static const char *init_hugepagefs(void)
> {
> +#ifdef CONFIG_LINUX
> + 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;
> }
I think I'd rather replace the whole if-statement with something like
this instead:
g_assert_cmpint(access(path, R_OK | W_OK | X_OK), ==, 0);
> @@ -482,17 +488,21 @@ 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;
> }
Maybe simplify to:
TFR(statfs_err = statfs(path, &fs));
g_assert_cmpint(statfs_err, ==, 0);
> if (fs.f_type != HUGETLBFS_MAGIC) {
> g_test_message("Warning: path not on HugeTLBFS: %s\n", path);
> + abort();
> return NULL;
> }
g_assert_cmpint(fs.f_type, ==, HUGETLBFS_MAGIC);
?
> return path;
> -}
> +#else
> + return NULL;
> #endif
> +}
>
> static TestServer *test_server_new(const gchar *name)
> {
> @@ -986,7 +996,6 @@ static void test_multiqueue(void)
>
> int main(int argc, char **argv)
> {
> - const char *hugefs;
> int ret;
> char template[] = "/tmp/vhost-test-XXXXXX";
>
> @@ -1001,14 +1010,7 @@ int main(int argc, char **argv)
> }
> g_assert(tmpfs);
>
> - root = tmpfs;
> -#ifdef CONFIG_LINUX
> - hugefs = getenv("QTEST_HUGETLBFS_PATH");
> - if (hugefs) {
> - root = init_hugepagefs(hugefs);
> - g_assert(root);
> - }
> -#endif
> + root = init_hugepagefs() ? : tmpfs;
I'd prefer the Elvis operator without space inbetween.
> if (qemu_memfd_check(0)) {
> qtest_add_data_func("/vhost-user/read-guest-mem/memfd",
Anyway, just nits, so still:
Reviewed-by: Thomas Huth <address@hidden>
- [Qemu-devel] [PATCH v2 0/9] vhost: enable for all targets, Paolo Bonzini, 2019/02/14
- [Qemu-devel] [PATCH 3/9] vhost: restrict Linux dependency to kernel vhost, Paolo Bonzini, 2019/02/14
- [Qemu-devel] [PATCH 4/9] vhost-user: support cross-endian vnet headers, Paolo Bonzini, 2019/02/14
- [Qemu-devel] [PATCH 9/9] vhost-user-test: create a temporary directory per TestServer, Paolo Bonzini, 2019/02/14
- [Qemu-devel] [PATCH 8/9] vhost-user-test: small changes to init_hugepagefs, Paolo Bonzini, 2019/02/14
- Re: [Qemu-devel] [PATCH 8/9] vhost-user-test: small changes to init_hugepagefs,
Thomas Huth <=
- [Qemu-devel] [PATCH 2/9] vhost-net-user: add stubs for when no virtio-net device is present, Paolo Bonzini, 2019/02/14
- [Qemu-devel] [PATCH 6/9] vhost-net: revamp configure logic, Paolo Bonzini, 2019/02/14
- [Qemu-devel] [PATCH 5/9] vhost-net: compile it on all targets that have virtio-net., Paolo Bonzini, 2019/02/14
- [Qemu-devel] [PATCH 1/9] vhost-net: move stubs to a separate file, Paolo Bonzini, 2019/02/14
- [Qemu-devel] [PATCH 7/9] vhost-user-test: create a main loop per TestServer, Paolo Bonzini, 2019/02/14
- Re: [Qemu-devel] [PATCH v2 0/9] vhost: enable for all targets, Michael S. Tsirkin, 2019/02/14
- Re: [Qemu-devel] [PATCH v2 0/9] vhost: enable for all targets, Michael S. Tsirkin, 2019/02/14