[libvirt] [PATCH 0/3] Rest of qemu agent tests and fix of expensive tests
This series contains the rest of the qemu agent tests that have been tweaked to act according to the VIR_TEST_EXPENSIVE env variable and also uses it for the virsh-all test Peter Krempa (3): qemuagenttest: Test arbitrary command passthrough qemuagenttest: Test timeout of agent commands test-lib.sh: Update helper for VIR_TEST_EXPENSIVE and use it in virsh-all tests/qemuagenttest.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++ tests/test-lib.sh | 10 ++--- tests/virsh-all | 2 + 3 files changed, 112 insertions(+), 5 deletions(-) -- 1.8.3.2
Excercise the arbitrary command passthrough API. --- tests/qemuagenttest.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c index cabd5b7..1818d50 100644 --- a/tests/qemuagenttest.c +++ b/tests/qemuagenttest.c @@ -444,6 +444,50 @@ cleanup: } +static const char testQemuAgentArbitraryCommandResponse[] = + "{\"return\":\"bla\"}"; + +static int +testQemuAgentArbitraryCommand(const void *data) +{ + virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data; + qemuMonitorTestPtr test = qemuMonitorTestNewAgent(xmlopt); + int ret = -1; + char *reply = NULL; + + if (!test) + return -1; + + if (qemuMonitorTestAddAgentSyncResponse(test) < 0) + goto cleanup; + + if (qemuMonitorTestAddItem(test, "ble", + testQemuAgentArbitraryCommandResponse) < 0) + goto cleanup; + + if (qemuAgentArbitraryCommand(qemuMonitorTestGetAgent(test), + "{\"execute\":\"ble\"}", + &reply, + VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0) + goto cleanup; + + if (STRNEQ(reply, testQemuAgentArbitraryCommandResponse)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "invalid processing of guest agent reply: " + "got '%s' expected '%s'", + reply, testQemuAgentArbitraryCommandResponse); + goto cleanup; + } + + ret = 0; + +cleanup: + VIR_FREE(reply); + qemuMonitorTestFree(test); + return ret; +} + + static int mymain(void) { @@ -471,6 +515,7 @@ mymain(void) DO_TEST(Suspend); DO_TEST(Shutdown); DO_TEST(CPU); + DO_TEST(ArbitraryCommand); virObjectUnref(xmlopt); -- 1.8.3.2
On 08/01/2013 07:24 AM, Peter Krempa wrote:
Excercise the arbitrary command passthrough API.
s/Excercise/Exercise/
--- tests/qemuagenttest.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 08/02/13 23:13, Eric Blake wrote:
On 08/01/2013 07:24 AM, Peter Krempa wrote:
Excercise the arbitrary command passthrough API.
s/Excercise/Exercise/
--- tests/qemuagenttest.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+)
ACK.
Pushed. Thanks. Peter
If VIR_TEST_EXPENSIVE is enabled, test timeout of agent commands. This test takes 6 seconds to finish. --- tests/qemuagenttest.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c index 1818d50..4b6392b 100644 --- a/tests/qemuagenttest.c +++ b/tests/qemuagenttest.c @@ -489,6 +489,65 @@ cleanup: static int +qemuAgentTimeoutTestMonitorHandler(qemuMonitorTestPtr test ATTRIBUTE_UNUSED, + qemuMonitorTestItemPtr item ATTRIBUTE_UNUSED, + const char *cmdstr ATTRIBUTE_UNUSED) +{ + return 0; +} + + +static int +testQemuAgentTimeout(const void *data) +{ + virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data; + qemuMonitorTestPtr test = qemuMonitorTestNewAgent(xmlopt); + char *reply = NULL; + int ret = -1; + + if (!test) + return -1; + + if (!getenv("VIR_TEST_EXPENSIVE")) + return EXIT_AM_SKIP; + + if (qemuMonitorTestAddHandler(test, qemuAgentTimeoutTestMonitorHandler, + NULL, NULL) < 0) + goto cleanup; + + if (qemuAgentFSFreeze(qemuMonitorTestGetAgent(test)) != -1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + "agent command should have failed"); + goto cleanup; + } + + /* test timeout */ + if (qemuMonitorTestAddAgentSyncResponse(test) < 0) + goto cleanup; + + if (qemuMonitorTestAddHandler(test, qemuAgentTimeoutTestMonitorHandler, + NULL, NULL) < 0) + goto cleanup; + + if (qemuAgentArbitraryCommand(qemuMonitorTestGetAgent(test), + "{\"execute\":\"ble\"}", + &reply, + 1) != -2) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + "agent command didn't time out"); + goto cleanup; + } + + ret = 0; + +cleanup: + VIR_FREE(reply); + qemuMonitorTestFree(test); + return ret; +} + + +static int mymain(void) { int ret = 0; @@ -516,6 +575,7 @@ mymain(void) DO_TEST(Shutdown); DO_TEST(CPU); DO_TEST(ArbitraryCommand); + DO_TEST(Timeout); virObjectUnref(xmlopt); -- 1.8.3.2
On 08/01/2013 07:24 AM, Peter Krempa wrote:
If VIR_TEST_EXPENSIVE is enabled, test timeout of agent commands. This test takes 6 seconds to finish. --- tests/qemuagenttest.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+)
In my v2 patch for setting up VIR_TEST_EXPENSIVE, I guaranteed that: make VIR_TEST_EXPENSIVE=0 => getenv("VIR_TEST_EXPENSIVE") sees "0" make VIR_TEST_EXPENSIVE=1 => getenv("VIR_TEST_EXPENSIVE") sees "1" make => getenv("VIR_TEST_EXPENSIVE") sees either "0" or "1", based on configure options as well as a fourth possibility: cd tests; ./qemuagenttest => getenv("VIR_TEST_EXPENSIVE") sees whatever is in your environment (usually NULL)
+ +static int +testQemuAgentTimeout(const void *data) +{ + virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data; + qemuMonitorTestPtr test = qemuMonitorTestNewAgent(xmlopt); + char *reply = NULL; + int ret = -1; + + if (!test) + return -1; + + if (!getenv("VIR_TEST_EXPENSIVE")) + return EXIT_AM_SKIP;
Hence, this is prone to false negatives (when expensive tests are disabled, getenv still returns a value when invoked through make). I'd better add a patch 2/2 on my proposal that tweaks testutils.h to provide a helper function (so we aren't repeating the logic of HOW to tell if we are in expensive mode), and this patch should be rebased on top of that to just call the helper function. Everything else looked okay, so ACK once we get the expensive skipping mechanics sorted. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
When the test-lib for shell tests was introduced it did think of expensive tests although this option was never used. Update the code for the new env variable name. Use this function in the virsh-all test that blindly runs all virsh commands without any arguments and thus it's rather time consuming than useful. Mark it as expensive to skip this test in normal cases. --- tests/test-lib.sh | 10 +++++----- tests/virsh-all | 2 ++ 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/test-lib.sh b/tests/test-lib.sh index 918bf73..6c5049a 100644 --- a/tests/test-lib.sh +++ b/tests/test-lib.sh @@ -158,15 +158,15 @@ require_selinux_() esac } -very_expensive_() +test_expensive() { - if test "$RUN_VERY_EXPENSIVE_TESTS" != yes; then + if test -z "$VIR_TEST_EXPENSIVE"; then skip_test_ ' This test is very expensive, so it is disabled by default. -To run it anyway, rerun make check with the RUN_VERY_EXPENSIVE_TESTS -environment variable set to yes. E.g., +To run it anyway, rerun make check with the VIR_TEST_EXPENSIVE +environment variable se. E.g., - env RUN_VERY_EXPENSIVE_TESTS=yes make check + env RUN_VERY_EXPENSIVE_TESTS=1 make check ' fi } diff --git a/tests/virsh-all b/tests/virsh-all index d4e2633..4e456c6 100755 --- a/tests/virsh-all +++ b/tests/virsh-all @@ -21,6 +21,8 @@ test -z "$srcdir" && srcdir=$(pwd) . "$srcdir/test-lib.sh" +test_expensive + fail=0 test_url=test:///default -- 1.8.3.2
On 08/01/2013 07:24 AM, Peter Krempa wrote:
When the test-lib for shell tests was introduced it did think of expensive tests although this option was never used.
More historically accurate: the shell script was lifted verbatim from GNU coreutils, back in the days when Jim Meyering was actively contributing here, where coreutils DOES have expensive tests. We have just never marked a test expensive in libvirt until now. But our test driver shell script diverged from coreutils long enough ago that I'm fine tweaking the script for our own needs instead of trying to resync from coreutils, so making further tweaks isn't going to make it harder to converge.
Update the code for the new env variable name.
Use this function in the virsh-all test that blindly runs all virsh commands without any arguments and thus it's rather time consuming than
s/rather/more/
useful. Mark it as expensive to skip this test in normal cases. --- tests/test-lib.sh | 10 +++++----- tests/virsh-all | 2 ++ 2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/tests/test-lib.sh b/tests/test-lib.sh index 918bf73..6c5049a 100644 --- a/tests/test-lib.sh +++ b/tests/test-lib.sh @@ -158,15 +158,15 @@ require_selinux_() esac }
-very_expensive_() +test_expensive() { - if test "$RUN_VERY_EXPENSIVE_TESTS" != yes; then + if test -z "$VIR_TEST_EXPENSIVE"; then
This test isn't quite right compared to the semantics I gave VIR_TEST_EXPENSIVE. I think I'll just fold the correct semantics into my 2/2 patch on that series.
skip_test_ ' This test is very expensive, so it is disabled by default. -To run it anyway, rerun make check with the RUN_VERY_EXPENSIVE_TESTS -environment variable set to yes. E.g., +To run it anyway, rerun make check with the VIR_TEST_EXPENSIVE +environment variable se. E.g.,
- env RUN_VERY_EXPENSIVE_TESTS=yes make check + env RUN_VERY_EXPENSIVE_TESTS=1 make check
Wrong variable name :) But again, I'll cover this part.
' fi } diff --git a/tests/virsh-all b/tests/virsh-all index d4e2633..4e456c6 100755 --- a/tests/virsh-all +++ b/tests/virsh-all @@ -21,6 +21,8 @@ test -z "$srcdir" && srcdir=$(pwd)
. "$srcdir/test-lib.sh"
+test_expensive +
That leaves just this part of your patch, once you rebase on top of my fixes. I agree with making this test marked expensive, and with doing it as a separate patch on top of the framework, so ACK to this portion once rebased on top of my patches. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 08/02/13 23:34, Eric Blake wrote:
On 08/01/2013 07:24 AM, Peter Krempa wrote:
When the test-lib for shell tests was introduced it did think of expensive tests although this option was never used.
More historically accurate: the shell script was lifted verbatim from GNU coreutils, back in the days when Jim Meyering was actively contributing here, where coreutils DOES have expensive tests. We have just never marked a test expensive in libvirt until now. But our test driver shell script diverged from coreutils long enough ago that I'm fine tweaking the script for our own needs instead of trying to resync from coreutils, so making further tweaks isn't going to make it harder to converge.
Update the code for the new env variable name.
Use this function in the virsh-all test that blindly runs all virsh commands without any arguments and thus it's rather time consuming than
s/rather/more/
useful. Mark it as expensive to skip this test in normal cases. --- tests/test-lib.sh | 10 +++++----- tests/virsh-all | 2 ++ 2 files changed, 7 insertions(+), 5 deletions(-)
...
That leaves just this part of your patch, once you rebase on top of my fixes. I agree with making this test marked expensive, and with doing it as a separate patch on top of the framework, so ACK to this portion once rebased on top of my patches.
Now that you've pushed the patches of yours I've pushed the rest of this series after rebasing. Peter
participants (2)
-
Eric Blake -
Peter Krempa