* [Qemu-devel] [PATCH] tests: use static qga config file
@ 2016-06-13 12:25 marcandre.lureau
2016-06-13 12:30 ` Marc-André Lureau
0 siblings, 1 reply; 3+ messages in thread
From: marcandre.lureau @ 2016-06-13 12:25 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, mdroth, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Do not create a leaking temporary file, but use a static file instead.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reported-by: Peter Maydell <peter.maydell@linaro.org>
---
tests/test-qga-config | 8 ++++++++
tests/test-qga.c | 27 ++++-----------------------
2 files changed, 12 insertions(+), 23 deletions(-)
create mode 100644 tests/test-qga-config
diff --git a/tests/test-qga-config b/tests/test-qga-config
new file mode 100644
index 0000000..4bb721a
--- /dev/null
+++ b/tests/test-qga-config
@@ -0,0 +1,8 @@
+[general]
+daemon=false
+method=virtio-serial
+path=/path/to/org.qemu.guest_agent.0
+pidfile=/var/foo/qemu-ga.pid
+statedir=/var/state
+verbose=true
+blacklist=guest-ping;guest-get-time
diff --git a/tests/test-qga.c b/tests/test-qga.c
index 251b201..7d206ec 100644
--- a/tests/test-qga.c
+++ b/tests/test-qga.c
@@ -691,28 +691,11 @@ static void test_qga_blacklist(gconstpointer data)
static void test_qga_config(gconstpointer data)
{
GError *error = NULL;
- char *cwd, *cmd, *out, *err, *str, **strv, *conf, **argv = NULL;
+ char *cwd, *cmd, *out, *err, *str, **strv, **argv = NULL;
char *env[2];
- int status, tmp;
+ int status;
gsize n;
GKeyFile *kf;
- const char *qga_config =
- "[general]\n"
- "daemon=false\n"
- "method=virtio-serial\n"
- "path=/path/to/org.qemu.guest_agent.0\n"
- "pidfile=/var/foo/qemu-ga.pid\n"
- "statedir=/var/state\n"
- "verbose=true\n"
- "blacklist=guest-ping;guest-get-time\n";
-
- tmp = g_file_open_tmp(NULL, &conf, &error);
- g_assert_no_error(error);
- g_assert_cmpint(tmp, >=, 0);
- g_assert_cmpstr(conf, !=, "");
-
- g_file_set_contents(conf, qga_config, -1, &error);
- g_assert_no_error(error);
cwd = g_get_current_dir();
cmd = g_strdup_printf("%s%cqemu-ga -D",
@@ -720,7 +703,8 @@ static void test_qga_config(gconstpointer data)
g_shell_parse_argv(cmd, NULL, &argv, &error);
g_assert_no_error(error);
- env[0] = g_strdup_printf("QGA_CONF=%s", conf);
+ env[0] = g_strdup_printf("QGA_CONF=%s%ctests%ctest-qga-config",
+ cwd, G_DIR_SEPARATOR, G_DIR_SEPARATOR);
env[1] = NULL;
g_spawn_sync(NULL, argv, env, 0,
NULL, NULL, &out, &err, &status, &error);
@@ -775,11 +759,8 @@ static void test_qga_config(gconstpointer data)
g_free(out);
g_free(err);
- g_free(conf);
g_free(env[0]);
g_key_file_free(kf);
-
- close(tmp);
}
static void test_qga_fsfreeze_status(gconstpointer fix)
--
2.7.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] tests: use static qga config file
2016-06-13 12:25 [Qemu-devel] [PATCH] tests: use static qga config file marcandre.lureau
@ 2016-06-13 12:30 ` Marc-André Lureau
2016-06-13 16:08 ` Michael Roth
0 siblings, 1 reply; 3+ messages in thread
From: Marc-André Lureau @ 2016-06-13 12:30 UTC (permalink / raw)
To: marcandre lureau; +Cc: qemu-devel, peter maydell, mdroth
Hi
----- Original Message -----
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Do not create a leaking temporary file, but use a static file instead.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> tests/test-qga-config | 8 ++++++++
> tests/test-qga.c | 27 ++++-----------------------
> 2 files changed, 12 insertions(+), 23 deletions(-)
> create mode 100644 tests/test-qga-config
>
> diff --git a/tests/test-qga-config b/tests/test-qga-config
> new file mode 100644
> index 0000000..4bb721a
> --- /dev/null
> +++ b/tests/test-qga-config
> @@ -0,0 +1,8 @@
> +[general]
> +daemon=false
> +method=virtio-serial
> +path=/path/to/org.qemu.guest_agent.0
> +pidfile=/var/foo/qemu-ga.pid
> +statedir=/var/state
> +verbose=true
> +blacklist=guest-ping;guest-get-time
> diff --git a/tests/test-qga.c b/tests/test-qga.c
> index 251b201..7d206ec 100644
> --- a/tests/test-qga.c
> +++ b/tests/test-qga.c
> @@ -691,28 +691,11 @@ static void test_qga_blacklist(gconstpointer data)
> static void test_qga_config(gconstpointer data)
> {
> GError *error = NULL;
> - char *cwd, *cmd, *out, *err, *str, **strv, *conf, **argv = NULL;
> + char *cwd, *cmd, *out, *err, *str, **strv, **argv = NULL;
> char *env[2];
> - int status, tmp;
> + int status;
> gsize n;
> GKeyFile *kf;
> - const char *qga_config =
> - "[general]\n"
> - "daemon=false\n"
> - "method=virtio-serial\n"
> - "path=/path/to/org.qemu.guest_agent.0\n"
> - "pidfile=/var/foo/qemu-ga.pid\n"
> - "statedir=/var/state\n"
> - "verbose=true\n"
> - "blacklist=guest-ping;guest-get-time\n";
> -
> - tmp = g_file_open_tmp(NULL, &conf, &error);
> - g_assert_no_error(error);
> - g_assert_cmpint(tmp, >=, 0);
> - g_assert_cmpstr(conf, !=, "");
> -
> - g_file_set_contents(conf, qga_config, -1, &error);
> - g_assert_no_error(error);
>
> cwd = g_get_current_dir();
> cmd = g_strdup_printf("%s%cqemu-ga -D",
> @@ -720,7 +703,8 @@ static void test_qga_config(gconstpointer data)
> g_shell_parse_argv(cmd, NULL, &argv, &error);
> g_assert_no_error(error);
>
> - env[0] = g_strdup_printf("QGA_CONF=%s", conf);
> + env[0] = g_strdup_printf("QGA_CONF=%s%ctests%ctest-qga-config",
> + cwd, G_DIR_SEPARATOR, G_DIR_SEPARATOR);
Actually instead of cwd here, it would be nice to have srcdir or testdir. There is g_test_build_filename() in glib 2.38, not sure what we should do without it. Ideas?
> env[1] = NULL;
> g_spawn_sync(NULL, argv, env, 0,
> NULL, NULL, &out, &err, &status, &error);
> @@ -775,11 +759,8 @@ static void test_qga_config(gconstpointer data)
>
> g_free(out);
> g_free(err);
> - g_free(conf);
> g_free(env[0]);
> g_key_file_free(kf);
> -
> - close(tmp);
> }
>
> static void test_qga_fsfreeze_status(gconstpointer fix)
> --
> 2.7.4
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] tests: use static qga config file
2016-06-13 12:30 ` Marc-André Lureau
@ 2016-06-13 16:08 ` Michael Roth
0 siblings, 0 replies; 3+ messages in thread
From: Michael Roth @ 2016-06-13 16:08 UTC (permalink / raw)
To: Marc-André Lureau, marcandre lureau; +Cc: qemu-devel, peter maydell
Quoting Marc-André Lureau (2016-06-13 07:30:59)
> Hi
>
> ----- Original Message -----
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Do not create a leaking temporary file, but use a static file instead.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Reported-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> > tests/test-qga-config | 8 ++++++++
> > tests/test-qga.c | 27 ++++-----------------------
> > 2 files changed, 12 insertions(+), 23 deletions(-)
> > create mode 100644 tests/test-qga-config
> >
> > diff --git a/tests/test-qga-config b/tests/test-qga-config
> > new file mode 100644
> > index 0000000..4bb721a
> > --- /dev/null
> > +++ b/tests/test-qga-config
> > @@ -0,0 +1,8 @@
> > +[general]
> > +daemon=false
> > +method=virtio-serial
> > +path=/path/to/org.qemu.guest_agent.0
> > +pidfile=/var/foo/qemu-ga.pid
> > +statedir=/var/state
> > +verbose=true
> > +blacklist=guest-ping;guest-get-time
> > diff --git a/tests/test-qga.c b/tests/test-qga.c
> > index 251b201..7d206ec 100644
> > --- a/tests/test-qga.c
> > +++ b/tests/test-qga.c
> > @@ -691,28 +691,11 @@ static void test_qga_blacklist(gconstpointer data)
> > static void test_qga_config(gconstpointer data)
> > {
> > GError *error = NULL;
> > - char *cwd, *cmd, *out, *err, *str, **strv, *conf, **argv = NULL;
> > + char *cwd, *cmd, *out, *err, *str, **strv, **argv = NULL;
> > char *env[2];
> > - int status, tmp;
> > + int status;
> > gsize n;
> > GKeyFile *kf;
> > - const char *qga_config =
> > - "[general]\n"
> > - "daemon=false\n"
> > - "method=virtio-serial\n"
> > - "path=/path/to/org.qemu.guest_agent.0\n"
> > - "pidfile=/var/foo/qemu-ga.pid\n"
> > - "statedir=/var/state\n"
> > - "verbose=true\n"
> > - "blacklist=guest-ping;guest-get-time\n";
> > -
> > - tmp = g_file_open_tmp(NULL, &conf, &error);
> > - g_assert_no_error(error);
> > - g_assert_cmpint(tmp, >=, 0);
> > - g_assert_cmpstr(conf, !=, "");
> > -
> > - g_file_set_contents(conf, qga_config, -1, &error);
> > - g_assert_no_error(error);
> >
> > cwd = g_get_current_dir();
> > cmd = g_strdup_printf("%s%cqemu-ga -D",
> > @@ -720,7 +703,8 @@ static void test_qga_config(gconstpointer data)
> > g_shell_parse_argv(cmd, NULL, &argv, &error);
> > g_assert_no_error(error);
> >
> > - env[0] = g_strdup_printf("QGA_CONF=%s", conf);
> > + env[0] = g_strdup_printf("QGA_CONF=%s%ctests%ctest-qga-config",
> > + cwd, G_DIR_SEPARATOR, G_DIR_SEPARATOR);
>
> Actually instead of cwd here, it would be nice to have srcdir or testdir. There is g_test_build_filename() in glib 2.38, not sure what we should do without it. Ideas?
Maybe just via environment variables?
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -660,6 +662,7 @@ $(patsubst %, check-%, $(check-unit-y)): check-%: %
$(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,)
$(call quiet-command, \
MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$((RANDOM % 255 + 1))} \
+ QEMU_TEST_SRC_PATH=$(SRC_PATH)/tests \
gtester $(GTESTER_OPTIONS) -m=$(SPEED) $*,"GTESTER $*")
$(if $(CONFIG_GCOV),@for f in $(gcov-files-$(subst tests/,,$*)-y) $(gcov-files-generic-y); do \
echo Gcov report for $$f:;\
We seem to do the same with MALLOC_PERTURB_ already (though there's probably
less options there), and it's consistent with how we control
QGA_TEST_SIDE_EFFECTING currently.
Also, if we default to cwd things should still work for running the test
executable directly.
g_test_build_filename seems to do similar under the covers as well,
though it is a bit more tedious to have to add to every test target we
want access environment variables in. Would be nice if
tests/Makefile.include could do it automatically via common
dependencies, but I'm not sure if that's doable.
>
> > env[1] = NULL;
> > g_spawn_sync(NULL, argv, env, 0,
> > NULL, NULL, &out, &err, &status, &error);
> > @@ -775,11 +759,8 @@ static void test_qga_config(gconstpointer data)
> >
> > g_free(out);
> > g_free(err);
> > - g_free(conf);
> > g_free(env[0]);
> > g_key_file_free(kf);
> > -
> > - close(tmp);
> > }
> >
> > static void test_qga_fsfreeze_status(gconstpointer fix)
> > --
> > 2.7.4
> >
> >
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-06-13 16:08 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-06-13 12:25 [Qemu-devel] [PATCH] tests: use static qga config file marcandre.lureau
2016-06-13 12:30 ` Marc-André Lureau
2016-06-13 16:08 ` Michael Roth
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).