This commit adds a test to ensure `merge-output` functions as expected.
We also add a negative test to ensure we haven't regressed previous
functionality.
Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
---
tests/unit/test-qga.c | 128 ++++++++++++++++++++++++++++++++++++------
1 file changed, 111 insertions(+), 17 deletions(-)
diff --git a/tests/unit/test-qga.c b/tests/unit/test-qga.c
index 9d8e1d1cd3..0b3966024c 100644
--- a/tests/unit/test-qga.c
+++ b/tests/unit/test-qga.c
@@ -755,6 +755,31 @@ static void test_qga_fsfreeze_status(gconstpointer fix)
g_assert_cmpstr(status, ==, "thawed");
}
+static QDict *wait_for_guest_exec_completion(int fd, int64_t pid)
+{
+ QDict *ret = NULL;
+ int64_t now;
+ bool exited;
+ QDict *val;
+
+ now = g_get_monotonic_time();
+ do {
+ ret = qmp_fd(fd,
+ "{'execute': 'guest-exec-status',"
+ " 'arguments': { 'pid': %" PRId64 " } }", pid);
+ g_assert_nonnull(ret);
+ val = qdict_get_qdict(ret, "return");
+ exited = qdict_get_bool(val, "exited");
+ if (!exited) {
+ qobject_unref(ret);
+ }
+ } while (!exited &&
+ g_get_monotonic_time() < now + 5 * G_TIME_SPAN_SECOND);
+ g_assert(exited);
+
+ return ret;
+}
+
static void test_qga_guest_exec(gconstpointer fix)
{
const TestFixture *fixture = fix;
@@ -762,9 +787,8 @@ static void test_qga_guest_exec(gconstpointer fix)
QDict *val;
const gchar *out;
g_autofree guchar *decoded = NULL;
- int64_t pid, now, exitcode;
+ int64_t pid, exitcode;
gsize len;
- bool exited;
/* exec 'echo foo bar' */
ret = qmp_fd(fixture->fd, "{'execute': 'guest-exec', 'arguments': {"
@@ -777,21 +801,7 @@ static void test_qga_guest_exec(gconstpointer fix)
g_assert_cmpint(pid, >, 0);
qobject_unref(ret);
- /* wait for completion */
- now = g_get_monotonic_time();
- do {
- ret = qmp_fd(fixture->fd,
- "{'execute': 'guest-exec-status',"
- " 'arguments': { 'pid': %" PRId64 " } }", pid);
- g_assert_nonnull(ret);
- val = qdict_get_qdict(ret, "return");
- exited = qdict_get_bool(val, "exited");
- if (!exited) {
- qobject_unref(ret);
- }
- } while (!exited &&
- g_get_monotonic_time() < now + 5 * G_TIME_SPAN_SECOND);
- g_assert(exited);
+ ret = wait_for_guest_exec_completion(fixture->fd, pid);
/* check stdout */
exitcode = qdict_get_int(val, "exitcode");
@@ -802,6 +812,86 @@ static void test_qga_guest_exec(gconstpointer fix)
g_assert_cmpstr((char *)decoded, ==, "\" test_str \"");
}
+static void test_qga_guest_exec_output_no_merge(gconstpointer fix)
+{
+ const TestFixture *fixture = fix;
+ g_autoptr(QDict) ret = NULL;
+ QDict *val;
+ const gchar *out, *err;
+ g_autofree guchar *out_decoded = NULL;
+ g_autofree guchar *err_decoded = NULL;
+ int64_t pid, exitcode;
+ gsize len;
+
+ /* exec 'echo foo bar' */
+ ret = qmp_fd(fixture->fd, "{'execute': 'guest-exec', 'arguments': {"
+ " 'path': '/bin/bash',"
+ " 'arg': [ '-c', 'for i in $(seq 4); do if (( $i %% 2 )); then echo stdout; else echo stderr 1>&2; fi; done;' ],"
+ " 'capture-output': true } }");
+ g_assert_nonnull(ret);
+ qmp_assert_no_error(ret);
+ val = qdict_get_qdict(ret, "return");
+ pid = qdict_get_int(val, "pid");
+ g_assert_cmpint(pid, >, 0);
+ qobject_unref(ret);
+
+ ret = wait_for_guest_exec_completion(fixture->fd, pid);
+
+ exitcode = qdict_get_int(val, "exitcode");
+ g_assert_cmpint(exitcode, ==, 0);
+
+ /* check stdout */
+ out = qdict_get_str(val, "out-data");
+ out_decoded = g_base64_decode(out, &len);
+ g_assert_cmpint(len, ==, 14);
+ g_assert_cmpstr((char *)out_decoded, ==, "stdout\nstdout\n");
+
+ /* check stderr */
+ err = qdict_get_try_str(val, "err-data");
+ err_decoded = g_base64_decode(err, &len);
+ g_assert_cmpint(len, ==, 14);
+ g_assert_cmpstr((char *)err_decoded, ==, "stderr\nstderr\n");
+}
+
+static void test_qga_guest_exec_output_merge(gconstpointer fix)
+{
+ const TestFixture *fixture = fix;
+ g_autoptr(QDict) ret = NULL;
+ QDict *val;
+ const gchar *out, *err;
+ g_autofree guchar *decoded = NULL;
+ int64_t pid, exitcode;
+ gsize len;
+
+ /* exec 'echo foo bar' */
+ ret = qmp_fd(fixture->fd, "{'execute': 'guest-exec', 'arguments': {"
+ " 'path': '/bin/bash',"
+ " 'arg': [ '-c', 'for i in $(seq 4); do if (( $i %% 2 )); then echo stdout; else echo stderr 1>&2; fi; done;' ],"
+ " 'capture-output': true,"
+ " 'merge-output': true } }");
+ g_assert_nonnull(ret);
+ qmp_assert_no_error(ret);
+ val = qdict_get_qdict(ret, "return");
+ pid = qdict_get_int(val, "pid");
+ g_assert_cmpint(pid, >, 0);
+ qobject_unref(ret);
+
+ ret = wait_for_guest_exec_completion(fixture->fd, pid);
+
+ exitcode = qdict_get_int(val, "exitcode");
+ g_assert_cmpint(exitcode, ==, 0);
+
+ /* check stdout */
+ out = qdict_get_str(val, "out-data");
+ decoded = g_base64_decode(out, &len);
+ g_assert_cmpint(len, ==, 28);
+ g_assert_cmpstr((char *)decoded, ==, "stdout\nstderr\nstdout\nstderr\n");
+
+ /* check stderr */
+ err = qdict_get_try_str(val, "err-data");
+ g_assert_null(err);
+}
+
static void test_qga_guest_exec_invalid(gconstpointer fix)
{
const TestFixture *fixture = fix;
@@ -975,6 +1065,10 @@ int main(int argc, char **argv)
g_test_add_data_func("/qga/blockedrpcs", NULL, test_qga_blockedrpcs);
g_test_add_data_func("/qga/config", NULL, test_qga_config);
g_test_add_data_func("/qga/guest-exec", &fix, test_qga_guest_exec);
+ g_test_add_data_func("/qga/guest-exec-output-no-merge", &fix,
+ test_qga_guest_exec_output_no_merge);
+ g_test_add_data_func("/qga/guest-exec-output-merge", &fix,
+ test_qga_guest_exec_output_merge);
g_test_add_data_func("/qga/guest-exec-invalid", &fix,
test_qga_guest_exec_invalid);
g_test_add_data_func("/qga/guest-get-osinfo", &fix,
--
2.39.1Hi
On Fri, Feb 24, 2023 at 8:31 AM Daniel Xu <dxu@dxuuu.xyz> wrote:
>
> This commit adds a test to ensure `merge-output` functions as expected.
> We also add a negative test to ensure we haven't regressed previous
> functionality.
>
> Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
Please check your patch with ASAN, you have use after-free introduced
by this change:
==664972==ERROR: AddressSanitizer: heap-use-after-free on address
0x621000135028 at pc 0x55e617a38b39 bp 0x7fff7fe85390 sp
0x7fff7fe85388
READ of size 8 at 0x621000135028 thread T0
#0 0x55e617a38b38 in qdict_find ../qobject/qdict.c:96
#1 0x55e617a39bea in qdict_get ../qobject/qdict.c:164
#2 0x55e617a39bea in qdict_get_int ../qobject/qdict.c:209
#3 0x55e6179e2519 in test_qga_guest_exec ../tests/unit/test-qga.c:807
#4 0x7fbaa499dc7d in g_test_run_suite_internal
(/lib64/libglib-2.0.so.0+0x84c7d)
#5 0x7fbaa499d9e4 in g_test_run_suite_internal
(/lib64/libglib-2.0.so.0+0x849e4)
#6 0x7fbaa499e181 in g_test_run_suite (/lib64/libglib-2.0.so.0+0x85181)
#7 0x7fbaa49966ec in g_test_run (/lib64/libglib-2.0.so.0+0x7d6ec)
#8 0x55e6179da0ac in main ../tests/unit/test-qga.c:1083
#9 0x7fbaa384a50f in __libc_start_call_main (/lib64/libc.so.6+0x2750f)
#10 0x7fbaa384a5c8 in __libc_start_main@GLIBC_2.2.5
(/lib64/libc.so.6+0x275c8)
#11 0x55e6179daf44 in _start
(/home/elmarco/src/qemu/build/tests/unit/test-qga+0x1bbf44)
thanks
> ---
> tests/unit/test-qga.c | 128 ++++++++++++++++++++++++++++++++++++------
> 1 file changed, 111 insertions(+), 17 deletions(-)
>
> diff --git a/tests/unit/test-qga.c b/tests/unit/test-qga.c
> index 9d8e1d1cd3..0b3966024c 100644
> --- a/tests/unit/test-qga.c
> +++ b/tests/unit/test-qga.c
> @@ -755,6 +755,31 @@ static void test_qga_fsfreeze_status(gconstpointer fix)
> g_assert_cmpstr(status, ==, "thawed");
> }
>
> +static QDict *wait_for_guest_exec_completion(int fd, int64_t pid)
> +{
> + QDict *ret = NULL;
> + int64_t now;
> + bool exited;
> + QDict *val;
> +
> + now = g_get_monotonic_time();
> + do {
> + ret = qmp_fd(fd,
> + "{'execute': 'guest-exec-status',"
> + " 'arguments': { 'pid': %" PRId64 " } }", pid);
> + g_assert_nonnull(ret);
> + val = qdict_get_qdict(ret, "return");
> + exited = qdict_get_bool(val, "exited");
> + if (!exited) {
> + qobject_unref(ret);
> + }
> + } while (!exited &&
> + g_get_monotonic_time() < now + 5 * G_TIME_SPAN_SECOND);
> + g_assert(exited);
> +
> + return ret;
> +}
> +
> static void test_qga_guest_exec(gconstpointer fix)
> {
> const TestFixture *fixture = fix;
> @@ -762,9 +787,8 @@ static void test_qga_guest_exec(gconstpointer fix)
> QDict *val;
> const gchar *out;
> g_autofree guchar *decoded = NULL;
> - int64_t pid, now, exitcode;
> + int64_t pid, exitcode;
> gsize len;
> - bool exited;
>
> /* exec 'echo foo bar' */
> ret = qmp_fd(fixture->fd, "{'execute': 'guest-exec', 'arguments': {"
> @@ -777,21 +801,7 @@ static void test_qga_guest_exec(gconstpointer fix)
> g_assert_cmpint(pid, >, 0);
> qobject_unref(ret);
>
> - /* wait for completion */
> - now = g_get_monotonic_time();
> - do {
> - ret = qmp_fd(fixture->fd,
> - "{'execute': 'guest-exec-status',"
> - " 'arguments': { 'pid': %" PRId64 " } }", pid);
> - g_assert_nonnull(ret);
> - val = qdict_get_qdict(ret, "return");
> - exited = qdict_get_bool(val, "exited");
> - if (!exited) {
> - qobject_unref(ret);
> - }
> - } while (!exited &&
> - g_get_monotonic_time() < now + 5 * G_TIME_SPAN_SECOND);
> - g_assert(exited);
> + ret = wait_for_guest_exec_completion(fixture->fd, pid);
>
> /* check stdout */
> exitcode = qdict_get_int(val, "exitcode");
> @@ -802,6 +812,86 @@ static void test_qga_guest_exec(gconstpointer fix)
> g_assert_cmpstr((char *)decoded, ==, "\" test_str \"");
> }
>
> +static void test_qga_guest_exec_output_no_merge(gconstpointer fix)
> +{
> + const TestFixture *fixture = fix;
> + g_autoptr(QDict) ret = NULL;
> + QDict *val;
> + const gchar *out, *err;
> + g_autofree guchar *out_decoded = NULL;
> + g_autofree guchar *err_decoded = NULL;
> + int64_t pid, exitcode;
> + gsize len;
> +
> + /* exec 'echo foo bar' */
> + ret = qmp_fd(fixture->fd, "{'execute': 'guest-exec', 'arguments': {"
> + " 'path': '/bin/bash',"
> + " 'arg': [ '-c', 'for i in $(seq 4); do if (( $i %% 2 )); then echo stdout; else echo stderr 1>&2; fi; done;' ],"
> + " 'capture-output': true } }");
> + g_assert_nonnull(ret);
> + qmp_assert_no_error(ret);
> + val = qdict_get_qdict(ret, "return");
> + pid = qdict_get_int(val, "pid");
> + g_assert_cmpint(pid, >, 0);
> + qobject_unref(ret);
> +
> + ret = wait_for_guest_exec_completion(fixture->fd, pid);
> +
> + exitcode = qdict_get_int(val, "exitcode");
> + g_assert_cmpint(exitcode, ==, 0);
> +
> + /* check stdout */
> + out = qdict_get_str(val, "out-data");
> + out_decoded = g_base64_decode(out, &len);
> + g_assert_cmpint(len, ==, 14);
> + g_assert_cmpstr((char *)out_decoded, ==, "stdout\nstdout\n");
> +
> + /* check stderr */
> + err = qdict_get_try_str(val, "err-data");
> + err_decoded = g_base64_decode(err, &len);
> + g_assert_cmpint(len, ==, 14);
> + g_assert_cmpstr((char *)err_decoded, ==, "stderr\nstderr\n");
> +}
> +
> +static void test_qga_guest_exec_output_merge(gconstpointer fix)
> +{
> + const TestFixture *fixture = fix;
> + g_autoptr(QDict) ret = NULL;
> + QDict *val;
> + const gchar *out, *err;
> + g_autofree guchar *decoded = NULL;
> + int64_t pid, exitcode;
> + gsize len;
> +
> + /* exec 'echo foo bar' */
> + ret = qmp_fd(fixture->fd, "{'execute': 'guest-exec', 'arguments': {"
> + " 'path': '/bin/bash',"
> + " 'arg': [ '-c', 'for i in $(seq 4); do if (( $i %% 2 )); then echo stdout; else echo stderr 1>&2; fi; done;' ],"
> + " 'capture-output': true,"
> + " 'merge-output': true } }");
> + g_assert_nonnull(ret);
> + qmp_assert_no_error(ret);
> + val = qdict_get_qdict(ret, "return");
> + pid = qdict_get_int(val, "pid");
> + g_assert_cmpint(pid, >, 0);
> + qobject_unref(ret);
> +
> + ret = wait_for_guest_exec_completion(fixture->fd, pid);
> +
> + exitcode = qdict_get_int(val, "exitcode");
> + g_assert_cmpint(exitcode, ==, 0);
> +
> + /* check stdout */
> + out = qdict_get_str(val, "out-data");
> + decoded = g_base64_decode(out, &len);
> + g_assert_cmpint(len, ==, 28);
> + g_assert_cmpstr((char *)decoded, ==, "stdout\nstderr\nstdout\nstderr\n");
> +
> + /* check stderr */
> + err = qdict_get_try_str(val, "err-data");
> + g_assert_null(err);
> +}
> +
> static void test_qga_guest_exec_invalid(gconstpointer fix)
> {
> const TestFixture *fixture = fix;
> @@ -975,6 +1065,10 @@ int main(int argc, char **argv)
> g_test_add_data_func("/qga/blockedrpcs", NULL, test_qga_blockedrpcs);
> g_test_add_data_func("/qga/config", NULL, test_qga_config);
> g_test_add_data_func("/qga/guest-exec", &fix, test_qga_guest_exec);
> + g_test_add_data_func("/qga/guest-exec-output-no-merge", &fix,
> + test_qga_guest_exec_output_no_merge);
> + g_test_add_data_func("/qga/guest-exec-output-merge", &fix,
> + test_qga_guest_exec_output_merge);
> g_test_add_data_func("/qga/guest-exec-invalid", &fix,
> test_qga_guest_exec_invalid);
> g_test_add_data_func("/qga/guest-get-osinfo", &fix,
> --
> 2.39.1
>
>
--
Marc-André Lureau
Hi, On Mon, Feb 27, 2023, at 1:40 AM, Marc-André Lureau wrote: > Hi > > On Fri, Feb 24, 2023 at 8:31 AM Daniel Xu <dxu@dxuuu.xyz> wrote: >> >> This commit adds a test to ensure `merge-output` functions as expected. >> We also add a negative test to ensure we haven't regressed previous >> functionality. >> >> Signed-off-by: Daniel Xu <dxu@dxuuu.xyz> > > Please check your patch with ASAN, you have use after-free introduced > by this change: > ==664972==ERROR: AddressSanitizer: heap-use-after-free on address > 0x621000135028 at pc 0x55e617a38b39 bp 0x7fff7fe85390 sp > 0x7fff7fe85388 > READ of size 8 at 0x621000135028 thread T0 > #0 0x55e617a38b38 in qdict_find ../qobject/qdict.c:96 > #1 0x55e617a39bea in qdict_get ../qobject/qdict.c:164 > #2 0x55e617a39bea in qdict_get_int ../qobject/qdict.c:209 > #3 0x55e6179e2519 in test_qga_guest_exec ../tests/unit/test-qga.c:807 > #4 0x7fbaa499dc7d in g_test_run_suite_internal > (/lib64/libglib-2.0.so.0+0x84c7d) > #5 0x7fbaa499d9e4 in g_test_run_suite_internal > (/lib64/libglib-2.0.so.0+0x849e4) > #6 0x7fbaa499e181 in g_test_run_suite (/lib64/libglib-2.0.so.0+0x85181) > #7 0x7fbaa49966ec in g_test_run (/lib64/libglib-2.0.so.0+0x7d6ec) > #8 0x55e6179da0ac in main ../tests/unit/test-qga.c:1083 > #9 0x7fbaa384a50f in __libc_start_call_main (/lib64/libc.so.6+0x2750f) > #10 0x7fbaa384a5c8 in __libc_start_main@GLIBC_2.2.5 > (/lib64/libc.so.6+0x275c8) > #11 0x55e6179daf44 in _start > (/home/elmarco/src/qemu/build/tests/unit/test-qga+0x1bbf44) Ack. Will fix. [...] Thanks, Daniel
© 2016 - 2025 Red Hat, Inc.