* [Qemu-devel] [PATCH 1/2] tests/tpm: fix tpm_util_swtpm_has_tpm2() @ 2018-10-03 13:21 Marc-André Lureau 2018-10-03 13:21 ` [Qemu-devel] [PATCH 2/2] tests/tpm: mark swtpm test as skipped instead of successful Marc-André Lureau 2018-10-03 14:21 ` [Qemu-devel] [PATCH 1/2] tests/tpm: fix tpm_util_swtpm_has_tpm2() Stefan Berger 0 siblings, 2 replies; 4+ messages in thread From: Marc-André Lureau @ 2018-10-03 13:21 UTC (permalink / raw) To: qemu-devel; +Cc: Marc-André Lureau, Stefan Berger Using g_spawn_async_with_pipes() is more complicated than running the sync version. The async version returns a file descriptor for stdout, which may not be fully read. Sometime "--tpm2" will failed to be read, and will cause the related test to be silently skipped. Use g_spawn_sync() instead, simplifying the code and fixing the race. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- tests/tpm-util.c | 46 ++++++++++++++++++++-------------------------- 1 file changed, 20 insertions(+), 26 deletions(-) diff --git a/tests/tpm-util.c b/tests/tpm-util.c index 9f3f156e42..ae4aaf35ca 100644 --- a/tests/tpm-util.c +++ b/tests/tpm-util.c @@ -145,39 +145,33 @@ void tpm_util_pcrread(QTestState *s, tx_func *tx, g_assert_cmpmem(buffer, exp_resp_size, exp_resp, exp_resp_size); } -static gboolean tpm_util_swtpm_has_tpm2(void) +static bool tpm_util_swtpm_has_tpm2(void) { - gint mystdout; - gboolean succ; - unsigned i; - char buffer[10240]; - ssize_t n; - gchar *swtpm_argv[] = { - g_strdup("swtpm"), g_strdup("socket"), g_strdup("--help"), NULL + bool has_tpm2 = false; + char *out = NULL; + static const char *argv[] = { + "swtpm", "socket", "--help", NULL }; - succ = g_spawn_async_with_pipes(NULL, swtpm_argv, NULL, - G_SPAWN_SEARCH_PATH, NULL, NULL, NULL, - NULL, &mystdout, NULL, NULL); - if (!succ) { - goto cleanup; + if (!g_spawn_sync(NULL /* working_dir */, + (char **)argv, + NULL /* envp */, + G_SPAWN_SEARCH_PATH, + NULL /* child_setup */, + NULL /* user_data */, + &out, + NULL /* err */, + NULL /* exit_status */, + NULL)) { + return false; } - n = read(mystdout, buffer, sizeof(buffer) - 1); - if (n < 0) { - goto cleanup; - } - buffer[n] = 0; - if (!strstr(buffer, "--tpm2")) { - succ = false; - } - - cleanup: - for (i = 0; swtpm_argv[i]; i++) { - g_free(swtpm_argv[i]); + if (strstr(out, "--tpm2")) { + has_tpm2 = true; } - return succ; + g_free(out); + return has_tpm2; } gboolean tpm_util_swtpm_start(const char *path, GPid *pid, -- 2.19.0.271.gfe8321ec05 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [Qemu-devel] [PATCH 2/2] tests/tpm: mark swtpm test as skipped instead of successful 2018-10-03 13:21 [Qemu-devel] [PATCH 1/2] tests/tpm: fix tpm_util_swtpm_has_tpm2() Marc-André Lureau @ 2018-10-03 13:21 ` Marc-André Lureau 2018-10-03 14:24 ` Stefan Berger 2018-10-03 14:21 ` [Qemu-devel] [PATCH 1/2] tests/tpm: fix tpm_util_swtpm_has_tpm2() Stefan Berger 1 sibling, 1 reply; 4+ messages in thread From: Marc-André Lureau @ 2018-10-03 13:21 UTC (permalink / raw) To: qemu-devel; +Cc: Marc-André Lureau, Stefan Berger If swtpm is not found in $PATH or --tpm2 isn't supported, let's mark the test as SKIP. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- tests/tpm-util.h | 2 ++ tests/tpm-tests.c | 33 +++++++++++++++++++++------------ tests/tpm-util.c | 8 +------- 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/tests/tpm-util.h b/tests/tpm-util.h index 330b9657fe..9e98bc5124 100644 --- a/tests/tpm-util.h +++ b/tests/tpm-util.h @@ -32,6 +32,8 @@ void tpm_util_pcrextend(QTestState *s, tx_func *tx); void tpm_util_pcrread(QTestState *s, tx_func *tx, const unsigned char *exp_resp, size_t exp_resp_size); +bool tpm_util_swtpm_has_tpm2(void); + gboolean tpm_util_swtpm_start(const char *path, GPid *pid, SocketAddress **addr, GError **error); void tpm_util_swtpm_kill(GPid pid); diff --git a/tests/tpm-tests.c b/tests/tpm-tests.c index 10c6592aac..e640777aa9 100644 --- a/tests/tpm-tests.c +++ b/tests/tpm-tests.c @@ -18,6 +18,17 @@ #include "libqtest.h" #include "tpm-tests.h" +static bool +tpm_test_swtpm_skip(void) +{ + if (!tpm_util_swtpm_has_tpm2()) { + g_test_skip("swtpm not in PATH or missing --tpm2 support"); + return true; + } + + return false; +} + void tpm_test_swtpm_test(const char *src_tpm_path, tx_func *tx, const char *ifmodel) { @@ -28,12 +39,13 @@ void tpm_test_swtpm_test(const char *src_tpm_path, tx_func *tx, GPid swtpm_pid; GError *error = NULL; - succ = tpm_util_swtpm_start(src_tpm_path, &swtpm_pid, &addr, &error); - /* succ may be false if swtpm is not available */ - if (!succ) { + if (tpm_test_swtpm_skip()) { return; } + succ = tpm_util_swtpm_start(src_tpm_path, &swtpm_pid, &addr, &error); + g_assert_true(succ); + args = g_strdup_printf( "-chardev socket,id=chr,path=%s " "-tpmdev emulator,id=dev,chardev=chr " @@ -74,19 +86,17 @@ void tpm_test_swtpm_migration_test(const char *src_tpm_path, GError *error = NULL; QTestState *src_qemu, *dst_qemu; - succ = tpm_util_swtpm_start(src_tpm_path, &src_tpm_pid, - &src_tpm_addr, &error); - /* succ may be false if swtpm is not available */ - if (!succ) { + if (tpm_test_swtpm_skip()) { return; } + succ = tpm_util_swtpm_start(src_tpm_path, &src_tpm_pid, + &src_tpm_addr, &error); + g_assert_true(succ); + succ = tpm_util_swtpm_start(dst_tpm_path, &dst_tpm_pid, &dst_tpm_addr, &error); - /* succ may be false if swtpm is not available */ - if (!succ) { - goto err_src_tpm_kill; - } + g_assert_true(succ); tpm_util_migration_start_qemu(&src_qemu, &dst_qemu, src_tpm_addr, dst_tpm_addr, uri, @@ -118,7 +128,6 @@ void tpm_test_swtpm_migration_test(const char *src_tpm_path, qapi_free_SocketAddress(dst_tpm_addr); } -err_src_tpm_kill: tpm_util_swtpm_kill(src_tpm_pid); if (src_tpm_addr) { g_unlink(src_tpm_addr->u.q_unix.path); diff --git a/tests/tpm-util.c b/tests/tpm-util.c index ae4aaf35ca..e08b137651 100644 --- a/tests/tpm-util.c +++ b/tests/tpm-util.c @@ -145,7 +145,7 @@ void tpm_util_pcrread(QTestState *s, tx_func *tx, g_assert_cmpmem(buffer, exp_resp_size, exp_resp, exp_resp_size); } -static bool tpm_util_swtpm_has_tpm2(void) +bool tpm_util_swtpm_has_tpm2(void) { bool has_tpm2 = false; char *out = NULL; @@ -190,11 +190,6 @@ gboolean tpm_util_swtpm_start(const char *path, GPid *pid, gboolean succ; unsigned i; - succ = tpm_util_swtpm_has_tpm2(); - if (!succ) { - goto cleanup; - } - *addr = g_new0(SocketAddress, 1); (*addr)->type = SOCKET_ADDRESS_TYPE_UNIX; (*addr)->u.q_unix.path = g_build_filename(path, "sock", NULL); @@ -202,7 +197,6 @@ gboolean tpm_util_swtpm_start(const char *path, GPid *pid, succ = g_spawn_async(NULL, swtpm_argv, NULL, G_SPAWN_SEARCH_PATH, NULL, NULL, pid, error); -cleanup: for (i = 0; swtpm_argv[i]; i++) { g_free(swtpm_argv[i]); } -- 2.19.0.271.gfe8321ec05 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] tests/tpm: mark swtpm test as skipped instead of successful 2018-10-03 13:21 ` [Qemu-devel] [PATCH 2/2] tests/tpm: mark swtpm test as skipped instead of successful Marc-André Lureau @ 2018-10-03 14:24 ` Stefan Berger 0 siblings, 0 replies; 4+ messages in thread From: Stefan Berger @ 2018-10-03 14:24 UTC (permalink / raw) To: Marc-André Lureau, qemu-devel On 10/3/18 9:21 AM, Marc-André Lureau wrote: > If swtpm is not found in $PATH or --tpm2 isn't supported, let's mark > the test as SKIP. > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > tests/tpm-util.h | 2 ++ > tests/tpm-tests.c | 33 +++++++++++++++++++++------------ > tests/tpm-util.c | 8 +------- > 3 files changed, 24 insertions(+), 19 deletions(-) > > diff --git a/tests/tpm-util.h b/tests/tpm-util.h > index 330b9657fe..9e98bc5124 100644 > --- a/tests/tpm-util.h > +++ b/tests/tpm-util.h > @@ -32,6 +32,8 @@ void tpm_util_pcrextend(QTestState *s, tx_func *tx); > void tpm_util_pcrread(QTestState *s, tx_func *tx, > const unsigned char *exp_resp, size_t exp_resp_size); > > +bool tpm_util_swtpm_has_tpm2(void); > + > gboolean tpm_util_swtpm_start(const char *path, GPid *pid, > SocketAddress **addr, GError **error); > void tpm_util_swtpm_kill(GPid pid); > diff --git a/tests/tpm-tests.c b/tests/tpm-tests.c > index 10c6592aac..e640777aa9 100644 > --- a/tests/tpm-tests.c > +++ b/tests/tpm-tests.c > @@ -18,6 +18,17 @@ > #include "libqtest.h" > #include "tpm-tests.h" > > +static bool > +tpm_test_swtpm_skip(void) > +{ > + if (!tpm_util_swtpm_has_tpm2()) { > + g_test_skip("swtpm not in PATH or missing --tpm2 support"); > + return true; > + } > + > + return false; > +} > + > void tpm_test_swtpm_test(const char *src_tpm_path, tx_func *tx, > const char *ifmodel) > { > @@ -28,12 +39,13 @@ void tpm_test_swtpm_test(const char *src_tpm_path, tx_func *tx, > GPid swtpm_pid; > GError *error = NULL; > > - succ = tpm_util_swtpm_start(src_tpm_path, &swtpm_pid, &addr, &error); > - /* succ may be false if swtpm is not available */ > - if (!succ) { > + if (tpm_test_swtpm_skip()) { > return; > } > > + succ = tpm_util_swtpm_start(src_tpm_path, &swtpm_pid, &addr, &error); > + g_assert_true(succ); > + > args = g_strdup_printf( > "-chardev socket,id=chr,path=%s " > "-tpmdev emulator,id=dev,chardev=chr " > @@ -74,19 +86,17 @@ void tpm_test_swtpm_migration_test(const char *src_tpm_path, > GError *error = NULL; > QTestState *src_qemu, *dst_qemu; > > - succ = tpm_util_swtpm_start(src_tpm_path, &src_tpm_pid, > - &src_tpm_addr, &error); > - /* succ may be false if swtpm is not available */ > - if (!succ) { > + if (tpm_test_swtpm_skip()) { > return; > } > > + succ = tpm_util_swtpm_start(src_tpm_path, &src_tpm_pid, > + &src_tpm_addr, &error); > + g_assert_true(succ); > + > succ = tpm_util_swtpm_start(dst_tpm_path, &dst_tpm_pid, > &dst_tpm_addr, &error); > - /* succ may be false if swtpm is not available */ > - if (!succ) { > - goto err_src_tpm_kill; > - } > + g_assert_true(succ); > > tpm_util_migration_start_qemu(&src_qemu, &dst_qemu, > src_tpm_addr, dst_tpm_addr, uri, > @@ -118,7 +128,6 @@ void tpm_test_swtpm_migration_test(const char *src_tpm_path, > qapi_free_SocketAddress(dst_tpm_addr); > } > > -err_src_tpm_kill: > tpm_util_swtpm_kill(src_tpm_pid); > if (src_tpm_addr) { > g_unlink(src_tpm_addr->u.q_unix.path); > diff --git a/tests/tpm-util.c b/tests/tpm-util.c > index ae4aaf35ca..e08b137651 100644 > --- a/tests/tpm-util.c > +++ b/tests/tpm-util.c > @@ -145,7 +145,7 @@ void tpm_util_pcrread(QTestState *s, tx_func *tx, > g_assert_cmpmem(buffer, exp_resp_size, exp_resp, exp_resp_size); > } > > -static bool tpm_util_swtpm_has_tpm2(void) > +bool tpm_util_swtpm_has_tpm2(void) > { > bool has_tpm2 = false; > char *out = NULL; > @@ -190,11 +190,6 @@ gboolean tpm_util_swtpm_start(const char *path, GPid *pid, > gboolean succ; > unsigned i; > > - succ = tpm_util_swtpm_has_tpm2(); > - if (!succ) { > - goto cleanup; > - } > - > *addr = g_new0(SocketAddress, 1); > (*addr)->type = SOCKET_ADDRESS_TYPE_UNIX; > (*addr)->u.q_unix.path = g_build_filename(path, "sock", NULL); > @@ -202,7 +197,6 @@ gboolean tpm_util_swtpm_start(const char *path, GPid *pid, > succ = g_spawn_async(NULL, swtpm_argv, NULL, G_SPAWN_SEARCH_PATH, > NULL, NULL, pid, error); > > -cleanup: > for (i = 0; swtpm_argv[i]; i++) { > g_free(swtpm_argv[i]); > } Reviewed-by: Stefan Berger <stefanb@linux.vnet.ibm.com> ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] tests/tpm: fix tpm_util_swtpm_has_tpm2() 2018-10-03 13:21 [Qemu-devel] [PATCH 1/2] tests/tpm: fix tpm_util_swtpm_has_tpm2() Marc-André Lureau 2018-10-03 13:21 ` [Qemu-devel] [PATCH 2/2] tests/tpm: mark swtpm test as skipped instead of successful Marc-André Lureau @ 2018-10-03 14:21 ` Stefan Berger 1 sibling, 0 replies; 4+ messages in thread From: Stefan Berger @ 2018-10-03 14:21 UTC (permalink / raw) To: Marc-André Lureau, qemu-devel On 10/3/18 9:21 AM, Marc-André Lureau wrote: > Using g_spawn_async_with_pipes() is more complicated than running the > sync version. The async version returns a file descriptor for stdout, which may > not be fully read. Sometime "--tpm2" will failed to be read, and will > cause the related test to be silently skipped. > > Use g_spawn_sync() instead, simplifying the code and fixing the race. > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > tests/tpm-util.c | 46 ++++++++++++++++++++-------------------------- > 1 file changed, 20 insertions(+), 26 deletions(-) > > diff --git a/tests/tpm-util.c b/tests/tpm-util.c > index 9f3f156e42..ae4aaf35ca 100644 > --- a/tests/tpm-util.c > +++ b/tests/tpm-util.c > @@ -145,39 +145,33 @@ void tpm_util_pcrread(QTestState *s, tx_func *tx, > g_assert_cmpmem(buffer, exp_resp_size, exp_resp, exp_resp_size); > } > > -static gboolean tpm_util_swtpm_has_tpm2(void) > +static bool tpm_util_swtpm_has_tpm2(void) > { > - gint mystdout; > - gboolean succ; > - unsigned i; > - char buffer[10240]; > - ssize_t n; > - gchar *swtpm_argv[] = { > - g_strdup("swtpm"), g_strdup("socket"), g_strdup("--help"), NULL > + bool has_tpm2 = false; > + char *out = NULL; > + static const char *argv[] = { > + "swtpm", "socket", "--help", NULL > }; > > - succ = g_spawn_async_with_pipes(NULL, swtpm_argv, NULL, > - G_SPAWN_SEARCH_PATH, NULL, NULL, NULL, > - NULL, &mystdout, NULL, NULL); > - if (!succ) { > - goto cleanup; > + if (!g_spawn_sync(NULL /* working_dir */, > + (char **)argv, > + NULL /* envp */, > + G_SPAWN_SEARCH_PATH, > + NULL /* child_setup */, > + NULL /* user_data */, > + &out, > + NULL /* err */, > + NULL /* exit_status */, > + NULL)) { > + return false; > } > > - n = read(mystdout, buffer, sizeof(buffer) - 1); > - if (n < 0) { > - goto cleanup; > - } > - buffer[n] = 0; > - if (!strstr(buffer, "--tpm2")) { > - succ = false; > - } > - > - cleanup: > - for (i = 0; swtpm_argv[i]; i++) { > - g_free(swtpm_argv[i]); > + if (strstr(out, "--tpm2")) { > + has_tpm2 = true; > } > > - return succ; > + g_free(out); > + return has_tpm2; > } > > gboolean tpm_util_swtpm_start(const char *path, GPid *pid, Reviewed-by: Stefan Berger <stefanb@linux.vnet.ibm.com> ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-10-03 14:25 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-10-03 13:21 [Qemu-devel] [PATCH 1/2] tests/tpm: fix tpm_util_swtpm_has_tpm2() Marc-André Lureau 2018-10-03 13:21 ` [Qemu-devel] [PATCH 2/2] tests/tpm: mark swtpm test as skipped instead of successful Marc-André Lureau 2018-10-03 14:24 ` Stefan Berger 2018-10-03 14:21 ` [Qemu-devel] [PATCH 1/2] tests/tpm: fix tpm_util_swtpm_has_tpm2() Stefan Berger
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).