9:39 a.m.
Fix the issues reported by John and also add yet another test subcase.
Peter Krempa (3):
qemuagenttest: Fix checking of shutdown mode
qemuagenttest: Check invalid response in shutdown test
qemumonitortestutils: Don't skip va_end() on error path
tests/qemuagenttest.c | 32 +++++++++++++++++++++++++++++++-
tests/qemumonitortestutils.c | 2 +-
2 files changed, 32 insertions(+), 2 deletions(-)
--
1.8.3.2
9:39 a.m.
New subject: [libvirt] [PATCH 1/3] qemuagenttest: Fix checking of shutdown mode
Coverity complained about unused variable that contains the shutdown
mode. The original intention was to check it against the requested mode.
Also the fixed check reveald a mistake in the expected shutdown mode.
Reported by John Ferlan.
---
tests/qemuagenttest.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c
index 4b6392b..e0df297 100644
--- a/tests/qemuagenttest.c
+++ b/tests/qemuagenttest.c
@@ -251,6 +251,13 @@ qemuAgentShutdownTestMonitorHandler(qemuMonitorTestPtr test,
goto cleanup;
}
+ if (STRNEQ(mode, data->mode)) {
+ ret = qemuMonitorReportError(test,
+ "expected shutdown mode '%s' got
'%s'",
+ data->mode, mode);
+ goto cleanup;
+ }
+
/* now don't reply but return a qemu agent event */
qemuAgentNotifyEvent(qemuMonitorTestGetAgent(test),
data->event);
@@ -279,7 +286,7 @@ testQemuAgentShutdown(const void *data)
goto cleanup;
priv.event = QEMU_AGENT_EVENT_SHUTDOWN;
- priv.mode = "shutdown";
+ priv.mode = "halt";
if (qemuMonitorTestAddHandler(test, qemuAgentShutdownTestMonitorHandler,
&priv, NULL) < 0)
--
1.8.3.2
12:46 p.m.
New subject: [libvirt] [PATCH 1/3] qemuagenttest: Fix checking of shutdown mode
On 08/01/2013 10:39 AM, Peter Krempa wrote:
Coverity complained about unused variable that contains the shutdown
mode. The original intention was to check it against the requested mode.
Also the fixed check reveald a mistake in the expected shutdown mode.
Reported by John Ferlan.
---
tests/qemuagenttest.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
ACK
John
12:53 p.m.
New subject: [libvirt] [PATCH 1/3] qemuagenttest: Fix checking of shutdown mode
On 08/01/2013 08:39 AM, Peter Krempa wrote:
Coverity complained about unused variable that contains the shutdown
mode. The original intention was to check it against the requested mode.
Also the fixed check reveald a mistake in the expected shutdown mode.
s/reveald/revealed/
Reported by John Ferlan.
---
tests/qemuagenttest.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
9:39 a.m.
New subject: [libvirt] [PATCH 2/3] qemuagenttest: Check invalid response in shutdown test
The shutdown test utilizes waiting for condition to exit the test. This
addition will return an error for the shutdown command to see if the
condition waiting code will not hang.
---
tests/qemuagenttest.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c
index e0df297..f889b93 100644
--- a/tests/qemuagenttest.c
+++ b/tests/qemuagenttest.c
@@ -324,6 +324,29 @@ testQemuAgentShutdown(const void *data)
QEMU_AGENT_SHUTDOWN_REBOOT) < 0)
goto cleanup;
+ /* check negative response, so that we can verify that the agent breaks
+ * out from sleep */
+
+ if (qemuMonitorTestAddAgentSyncResponse(test) < 0)
+ goto cleanup;
+
+ if (qemuMonitorTestAddItem(test, "guest-shutdown",
+ "{\"error\":"
+ "
{\"class\":\"CommandDisabled\","
+ " \"desc\":\"The command
guest-shutdown has "
+ "been disabled for this
instance\","
+ "
\"data\":{\"name\":\"guest-shutdown\"}"
+ " }"
+ "}") < 0)
+ goto cleanup;
+
+ if (qemuAgentShutdown(qemuMonitorTestGetAgent(test),
+ QEMU_AGENT_SHUTDOWN_REBOOT) != -1) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ "agent shutdown command should have failed");
+ goto cleanup;
+ }
+
ret = 0;
cleanup:
--
1.8.3.2
12:55 p.m.
New subject: [libvirt] [PATCH 2/3] qemuagenttest: Check invalid response in shutdown test
On 08/01/2013 08:39 AM, Peter Krempa wrote:
The shutdown test utilizes waiting for condition to exit the test.
This
addition will return an error for the shutdown command to see if the
condition waiting code will not hang.
---
tests/qemuagenttest.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
ACK - negative tests are important too :)
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
12:57 p.m.
New subject: [libvirt] [PATCH 2/3] qemuagenttest: Check invalid response in shutdown test
On 08/01/2013 10:39 AM, Peter Krempa wrote:
The shutdown test utilizes waiting for condition to exit the test.
This
addition will return an error for the shutdown command to see if the
condition waiting code will not hang.
---
tests/qemuagenttest.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
ACK (although running with VIR_TEST_DEBUG=1 results in a message being
printed which threw me at first).
John
1:01 p.m.
New subject: [libvirt] [PATCH 2/3] qemuagenttest: Check invalid response in shutdown test
On 08/01/2013 11:57 AM, John Ferlan wrote:
On 08/01/2013 10:39 AM, Peter Krempa wrote:
> The shutdown test utilizes waiting for condition to exit the test. This
> addition will return an error for the shutdown command to see if the
> condition waiting code will not hang.
> ---
> tests/qemuagenttest.c | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
ACK (although running with VIR_TEST_DEBUG=1 results in a message being
printed which threw me at first).
Do we need to call a reset function to clear the expected error?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
3:43 a.m.
New subject: [libvirt] [PATCH 2/3] qemuagenttest: Check invalid response in shutdown test
On 08/01/13 20:01, Eric Blake wrote:
On 08/01/2013 11:57 AM, John Ferlan wrote:
> On 08/01/2013 10:39 AM, Peter Krempa wrote:
>> The shutdown test utilizes waiting for condition to exit the test. This
>> addition will return an error for the shutdown command to see if the
>> condition waiting code will not hang.
>> ---
>> tests/qemuagenttest.c | 23 +++++++++++++++++++++++
>> 1 file changed, 23 insertions(+)
>>
> ACK (although running with VIR_TEST_DEBUG=1 results in a message being
> printed which threw me at first).
Do we need to call a reset function to clear the expected error?
This will probably warrant a separate refactor of the tests. There are a
few of those that already report an (expected) error and then end
successfully.
Peter
9:39 a.m.
New subject: [libvirt] [PATCH 3/3] qemumonitortestutils: Don't skip va_end() on error path
When allocation of memory failed the error path didn't call va_end()
causing a coverity failure.
Reported by John Ferlan.
---
tests/qemumonitortestutils.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c
index fb76156..5697946 100644
--- a/tests/qemumonitortestutils.c
+++ b/tests/qemumonitortestutils.c
@@ -669,7 +669,7 @@ qemuMonitorTestAddItemParams(qemuMonitorTestPtr test,
va_start(args, response);
if (VIR_ALLOC(data) < 0)
- return -1;
+ goto error;
if (VIR_STRDUP(data->command_name, cmdname) < 0 ||
VIR_STRDUP(data->response, response) < 0)
--
1.8.3.2
12:55 p.m.
New subject: [libvirt] [PATCH 3/3] qemumonitortestutils: Don't skip va_end() on error path
On 08/01/2013 08:39 AM, Peter Krempa wrote:
When allocation of memory failed the error path didn't call
va_end()
causing a coverity failure.
Reported by John Ferlan.
---
tests/qemumonitortestutils.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
ACK.
diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c
index fb76156..5697946 100644
--- a/tests/qemumonitortestutils.c
+++ b/tests/qemumonitortestutils.c
@@ -669,7 +669,7 @@ qemuMonitorTestAddItemParams(qemuMonitorTestPtr test,
va_start(args, response);
if (VIR_ALLOC(data) < 0)
- return -1;
+ goto error;
if (VIR_STRDUP(data->command_name, cmdname) < 0 ||
VIR_STRDUP(data->response, response) < 0)
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
12:57 p.m.
New subject: [libvirt] [PATCH 3/3] qemumonitortestutils: Don't skip va_end() on error path
On 08/01/2013 10:39 AM, Peter Krempa wrote:
When allocation of memory failed the error path didn't call
va_end()
causing a coverity failure.
Reported by John Ferlan.
---
tests/qemumonitortestutils.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
ACK
John
5:32 a.m.
New subject: [libvirt] [PATCH 3/3] qemumonitortestutils: Don't skip va_end() on error path
On 08/01/13 19:57, John Ferlan wrote:
On 08/01/2013 10:39 AM, Peter Krempa wrote:
> When allocation of memory failed the error path didn't call va_end()
> causing a coverity failure.
>
> Reported by John Ferlan.
> ---
> tests/qemumonitortestutils.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
ACK
John
Series pushed. Thanks for the reviews :)
Peter
4165
days inactive
4166
days old
12 comments
3 participants
participants (3)
-
Eric Blake -
John Ferlan -
Peter Krempa