* [Qemu-devel] [PATCH 0/2] Remove test-qga temporary file @ 2016-06-14 13:16 marcandre.lureau 2016-06-14 13:16 ` [Qemu-devel] [PATCH 1/2] build-sys: define QEMU_SRC_PATH marcandre.lureau 2016-06-14 13:16 ` [Qemu-devel] [PATCH 2/2] tests: use static qga config file marcandre.lureau 0 siblings, 2 replies; 5+ messages in thread From: marcandre.lureau @ 2016-06-14 13:16 UTC (permalink / raw) To: qemu-devel; +Cc: peter.maydell, mdroth, Marc-André Lureau From: Marc-André Lureau <marcandre.lureau@redhat.com> Hi, Here are 2 small patches to remove the need for temporary file creation in test-qga. Marc-André Lureau (2): build-sys: define QEMU_SRC_PATH tests: use static qga config file scripts/create_config | 3 +++ tests/test-qga-config | 8 ++++++++ tests/test-qga.c | 27 ++++----------------------- 3 files changed, 15 insertions(+), 23 deletions(-) create mode 100644 tests/test-qga-config -- 2.7.4 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH 1/2] build-sys: define QEMU_SRC_PATH 2016-06-14 13:16 [Qemu-devel] [PATCH 0/2] Remove test-qga temporary file marcandre.lureau @ 2016-06-14 13:16 ` marcandre.lureau 2016-06-14 21:59 ` Michael Roth 2016-06-14 13:16 ` [Qemu-devel] [PATCH 2/2] tests: use static qga config file marcandre.lureau 1 sibling, 1 reply; 5+ messages in thread From: marcandre.lureau @ 2016-06-14 13:16 UTC (permalink / raw) To: qemu-devel; +Cc: peter.maydell, mdroth, Marc-André Lureau From: Marc-André Lureau <marcandre.lureau@redhat.com> Define QEMU_SRC_PATH in config-host.h, to ease accessing of tests data files. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- scripts/create_config | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scripts/create_config b/scripts/create_config index 1dd6a35..2fbe126 100755 --- a/scripts/create_config +++ b/scripts/create_config @@ -116,6 +116,9 @@ case $line in DSOSUF=*) echo "#define HOST_DSOSUF \"${line#*=}\"" ;; + SRC_PATH=*) + echo "#define QEMU_SRC_PATH \"${line#*=}\"" + ;; esac done # read -- 2.7.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] build-sys: define QEMU_SRC_PATH 2016-06-14 13:16 ` [Qemu-devel] [PATCH 1/2] build-sys: define QEMU_SRC_PATH marcandre.lureau @ 2016-06-14 21:59 ` Michael Roth 2016-06-15 10:46 ` Marc-André Lureau 0 siblings, 1 reply; 5+ messages in thread From: Michael Roth @ 2016-06-14 21:59 UTC (permalink / raw) To: marcandre.lureau, qemu-devel; +Cc: peter.maydell Quoting marcandre.lureau@redhat.com (2016-06-14 08:16:53) > From: Marc-André Lureau <marcandre.lureau@redhat.com> > > Define QEMU_SRC_PATH in config-host.h, to ease accessing of tests data > files. > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> I know this avoids the need to define environment variables for individual test targets to pass on SRC_PATH, but the fact that we didn't rely on this before made me a bit apprehensive about suggesting it. qemu-iotests for instance relies on a symlink back to SRC_PATH, and check-qapi-schema tests feed individual schema files to a script via tests/Makefile.include. Both can be made to check against a different/changing SRC_PATH, but if we bake it into the code that's not possible. I'm not sure if there's a valid use-case for needing to do so, but it seems to be bad form to not have any mechanism to change them without recompiling. Personally I think I'd prefer the environment variable approach, even if it means needing to add it per-target. I think if we wanted to get fancy we could do this via a recipe that exports environment variables added to a lazily-evaluated Makefile variable by each target's dependencies, but it's probably not worth it outside of a more general cleanup to how we handle SRC_PATH/BUILD_DIR dependencies throughout tests/ If others are fine with the approach taken here though I wouldn't hold things up. > --- > scripts/create_config | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/scripts/create_config b/scripts/create_config > index 1dd6a35..2fbe126 100755 > --- a/scripts/create_config > +++ b/scripts/create_config > @@ -116,6 +116,9 @@ case $line in > DSOSUF=*) > echo "#define HOST_DSOSUF \"${line#*=}\"" > ;; > + SRC_PATH=*) > + echo "#define QEMU_SRC_PATH \"${line#*=}\"" > + ;; > esac > > done # read > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] build-sys: define QEMU_SRC_PATH 2016-06-14 21:59 ` Michael Roth @ 2016-06-15 10:46 ` Marc-André Lureau 0 siblings, 0 replies; 5+ messages in thread From: Marc-André Lureau @ 2016-06-15 10:46 UTC (permalink / raw) To: Michael Roth; +Cc: QEMU, Peter Maydell Hi On Tue, Jun 14, 2016 at 11:59 PM, Michael Roth <mdroth@linux.vnet.ibm.com> wrote: > Quoting marcandre.lureau@redhat.com (2016-06-14 08:16:53) >> From: Marc-André Lureau <marcandre.lureau@redhat.com> >> >> Define QEMU_SRC_PATH in config-host.h, to ease accessing of tests data >> files. >> >> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > I know this avoids the need to define environment variables for > individual test targets to pass on SRC_PATH, but the fact that > we didn't rely on this before made me a bit apprehensive about > suggesting it. qemu-iotests for instance relies on a symlink > back to SRC_PATH, and check-qapi-schema tests feed individual > schema files to a script via tests/Makefile.include. Both can > be made to check against a different/changing SRC_PATH, but > if we bake it into the code that's not possible. Right, so let's use a symlink too? +if [ ! -e tests/test-qga-config ]; then + symlink "$source_path/tests/test-qga-config" tests/test-qga-config +fi > > I'm not sure if there's a valid use-case for needing to do > so, but it seems to be bad form to not have any mechanism > to change them without recompiling. Personally I think I'd > prefer the environment variable approach, even if it means > needing to add it per-target. I think if we wanted to get > fancy we could do this via a recipe that exports environment > variables added to a lazily-evaluated Makefile variable by > each target's dependencies, but it's probably not worth it > outside of a more general cleanup to how we handle > SRC_PATH/BUILD_DIR dependencies throughout tests/ I agree an environment variable would be nice (along with helper functions to lookup test data files), but I think we should stick with the common symlink way for now. I'll resend the patch, thanks -- Marc-André Lureau ^ permalink raw reply [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH 2/2] tests: use static qga config file 2016-06-14 13:16 [Qemu-devel] [PATCH 0/2] Remove test-qga temporary file marcandre.lureau 2016-06-14 13:16 ` [Qemu-devel] [PATCH 1/2] build-sys: define QEMU_SRC_PATH marcandre.lureau @ 2016-06-14 13:16 ` marcandre.lureau 1 sibling, 0 replies; 5+ messages in thread From: marcandre.lureau @ 2016-06-14 13:16 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..8686c23 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", + QEMU_SRC_PATH, 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] 5+ messages in thread
end of thread, other threads:[~2016-06-15 10:46 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-06-14 13:16 [Qemu-devel] [PATCH 0/2] Remove test-qga temporary file marcandre.lureau 2016-06-14 13:16 ` [Qemu-devel] [PATCH 1/2] build-sys: define QEMU_SRC_PATH marcandre.lureau 2016-06-14 21:59 ` Michael Roth 2016-06-15 10:46 ` Marc-André Lureau 2016-06-14 13:16 ` [Qemu-devel] [PATCH 2/2] tests: use static qga config file marcandre.lureau
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.