[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] test-qga: Kill broken and dead QGA_TEST_SIDE_EF
From: |
Marc-André Lureau |
Subject: |
Re: [Qemu-devel] [PATCH] test-qga: Kill broken and dead QGA_TEST_SIDE_EFFECTING code |
Date: |
Wed, 2 Aug 2017 18:07:22 -0400 (EDT) |
----- Original Message -----
> Back when the test was introduced, in commit 62c39b307, the
> test was set up to run qemu-ga directly on the host performing
> the test, and defaults to limiting itself to safe commands. At
> the time, it was envisioned that setting QGA_TEST_SIDE_EFFECTING
> in the environment could cover a few more commands, while noting
> the potential danger of those side effects running in the host.
>
> But this has NEVER been tested: if you enable the environment
> variable, the test WILL fail. One obvious reason: if you are not
> running as root, you'll probably get a permission failure when
> trying to freeze the file systems, or when changing system time.
> Less obvious: if you run the test as root (wow, you're brave), you
> could end up hanging if the test tries to log things to a
> temporarily frozen filesystem. But the cutest reason of all: if
> you get past the above hurdles, the test uses invalid JSON in
> test_qga_fstrim() (missing '' around the dictionary key 'minimum'),
> and will thus fail an assertion in qmp_fd().
>
> Rather than leave this untested time-bomb in place, rip it out.
> Hopefully, as originally envisioned, we can find an opportunity
> to test an actual sandboxed guest where the guest-agent has
> full permissions and will not unduly affect the host running
> the test - if so, 'git revert' can be used if desired, for
> salvaging any useful parts of this attempt.
>
> Signed-off-by: Eric Blake <address@hidden>
I think it used to work in a vm (as qemu-ga user), but I don't mind to revert
it if we need it back:
Signed-off-by: Marc-André Lureau <address@hidden>
> ---
> tests/test-qga.c | 90
> --------------------------------------------------------
> 1 file changed, 90 deletions(-)
>
> diff --git a/tests/test-qga.c b/tests/test-qga.c
> index 06783e7585..fd6bc7690f 100644
> --- a/tests/test-qga.c
> +++ b/tests/test-qga.c
> @@ -642,65 +642,6 @@ static void test_qga_get_time(gconstpointer fix)
> QDECREF(ret);
> }
>
> -static void test_qga_set_time(gconstpointer fix)
> -{
> - const TestFixture *fixture = fix;
> - QDict *ret;
> - int64_t current, time;
> - gchar *cmd;
> -
> - /* get current time */
> - ret = qmp_fd(fixture->fd, "{'execute': 'guest-get-time'}");
> - g_assert_nonnull(ret);
> - qmp_assert_no_error(ret);
> - current = qdict_get_int(ret, "return");
> - g_assert_cmpint(current, >, 0);
> - QDECREF(ret);
> -
> - /* set some old time */
> - ret = qmp_fd(fixture->fd, "{'execute': 'guest-set-time',"
> - " 'arguments': { 'time': 1000 } }");
> - g_assert_nonnull(ret);
> - qmp_assert_no_error(ret);
> - QDECREF(ret);
> -
> - /* check old time */
> - ret = qmp_fd(fixture->fd, "{'execute': 'guest-get-time'}");
> - g_assert_nonnull(ret);
> - qmp_assert_no_error(ret);
> - time = qdict_get_int(ret, "return");
> - g_assert_cmpint(time / 1000, <, G_USEC_PER_SEC * 10);
> - QDECREF(ret);
> -
> - /* set back current time */
> - cmd = g_strdup_printf("{'execute': 'guest-set-time',"
> - " 'arguments': { 'time': %" PRId64 " } }",
> - current + time * 1000);
> - ret = qmp_fd(fixture->fd, cmd);
> - g_free(cmd);
> - g_assert_nonnull(ret);
> - qmp_assert_no_error(ret);
> - QDECREF(ret);
> -}
> -
> -static void test_qga_fstrim(gconstpointer fix)
> -{
> - const TestFixture *fixture = fix;
> - QDict *ret;
> - QList *list;
> - const QListEntry *entry;
> -
> - ret = qmp_fd(fixture->fd, "{'execute': 'guest-fstrim',"
> - " arguments: { minimum: 4194304 } }");
> - g_assert_nonnull(ret);
> - qmp_assert_no_error(ret);
> - list = qdict_get_qlist(ret, "return");
> - entry = qlist_first(list);
> - g_assert(qdict_haskey(qobject_to_qdict(entry->value), "paths"));
> -
> - QDECREF(ret);
> -}
> -
> static void test_qga_blacklist(gconstpointer data)
> {
> TestFixture fix;
> @@ -831,30 +772,6 @@ static void test_qga_fsfreeze_status(gconstpointer fix)
> QDECREF(ret);
> }
>
> -static void test_qga_fsfreeze_and_thaw(gconstpointer fix)
> -{
> - const TestFixture *fixture = fix;
> - QDict *ret;
> - const gchar *status;
> -
> - ret = qmp_fd(fixture->fd, "{'execute': 'guest-fsfreeze-freeze'}");
> - g_assert_nonnull(ret);
> - qmp_assert_no_error(ret);
> - QDECREF(ret);
> -
> - ret = qmp_fd(fixture->fd, "{'execute': 'guest-fsfreeze-status'}");
> - g_assert_nonnull(ret);
> - qmp_assert_no_error(ret);
> - status = qdict_get_try_str(ret, "return");
> - g_assert_cmpstr(status, ==, "frozen");
> - QDECREF(ret);
> -
> - ret = qmp_fd(fixture->fd, "{'execute': 'guest-fsfreeze-thaw'}");
> - g_assert_nonnull(ret);
> - qmp_assert_no_error(ret);
> - QDECREF(ret);
> -}
> -
> static void test_qga_guest_exec(gconstpointer fix)
> {
> const TestFixture *fixture = fix;
> @@ -1029,13 +946,6 @@ int main(int argc, char **argv)
> g_test_add_data_func("/qga/guest-get-osinfo", &fix,
> test_qga_guest_get_osinfo);
>
> - if (g_getenv("QGA_TEST_SIDE_EFFECTING")) {
> - g_test_add_data_func("/qga/fsfreeze-and-thaw", &fix,
> - test_qga_fsfreeze_and_thaw);
> - g_test_add_data_func("/qga/set-time", &fix, test_qga_set_time);
> - g_test_add_data_func("/qga/fstrim", &fix, test_qga_fstrim);
> - }
> -
> ret = g_test_run();
>
> fixture_tear_down(&fix, NULL);
> --
> 2.13.3
>
>