* [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 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
* 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
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 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.