[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v6 03/29] test-qga: Kill broken and dead QGA_TES
From: |
Thomas Huth |
Subject: |
Re: [Qemu-devel] [PATCH v6 03/29] test-qga: Kill broken and dead QGA_TEST_SIDE_EFFECTING code |
Date: |
Tue, 5 Sep 2017 09:43:48 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 |
On 01.09.2017 20:03, Eric Blake wrote:
> 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>
> Reviewed-by: Marc-André Lureau <address@hidden>
> ---
> tests/test-qga.c | 90
> --------------------------------------------------------
> 1 file changed, 90 deletions(-)
Reviewed-by: Thomas Huth <address@hidden>
- [Qemu-devel] [PATCH v6 00/29] Preliminary libqtest cleanups, Eric Blake, 2017/09/01
- [Qemu-devel] [PATCH v6 01/29] tests: Improve .gitignore for tests/multiboot, Eric Blake, 2017/09/01
- [Qemu-devel] [PATCH v6 03/29] test-qga: Kill broken and dead QGA_TEST_SIDE_EFFECTING code, Eric Blake, 2017/09/01
- Re: [Qemu-devel] [PATCH v6 03/29] test-qga: Kill broken and dead QGA_TEST_SIDE_EFFECTING code,
Thomas Huth <=
- [Qemu-devel] [PATCH v6 02/29] tests: Sort .gitignore, Eric Blake, 2017/09/01
- Re: [Qemu-devel] [PATCH v6 02/29] tests: Sort .gitignore, Thomas Huth, 2017/09/05
- Re: [Qemu-devel] [PATCH v6 02/29] tests: Sort .gitignore, Markus Armbruster, 2017/09/05
- Re: [Qemu-devel] [PATCH v6 02/29] tests: Sort .gitignore, Thomas Huth, 2017/09/05
- Re: [Qemu-devel] [PATCH v6 02/29] tests: Sort .gitignore, Daniel P. Berrange, 2017/09/05
- Re: [Qemu-devel] [PATCH v6 02/29] tests: Sort .gitignore, Thomas Huth, 2017/09/05
- Re: [Qemu-devel] [PATCH v6 02/29] tests: Sort .gitignore, Eric Blake, 2017/09/05
[Qemu-devel] [PATCH v6 05/29] numa-test: Use hmp(), Eric Blake, 2017/09/01
[Qemu-devel] [PATCH v6 04/29] qtest: Don't perform side effects inside assertion, Eric Blake, 2017/09/01