[libvirt] [PATCH 2/5] add qemuAgentCommand()
Add qemuAgentCommand() for general qemu agent command. include/libvirt/libvirt-qemu.h | 5 +++++ src/qemu/qemu_agent.c | 38 ++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_agent.h | 5 +++++ 3 files changed, 48 insertions(+) diff --git a/include/libvirt/libvirt-qemu.h b/include/libvirt/libvirt-qemu.h index a37f897..ffc5ae5 100644 --- a/include/libvirt/libvirt-qemu.h +++ b/include/libvirt/libvirt-qemu.h @@ -44,6 +44,11 @@ virDomainPtr virDomainQemuAttach(virConnectPtr domain, unsigned int pid_value, unsigned int flags); +typedef enum { + VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK = -1, + VIR_DOMAIN_QEMU_AGENT_COMMAND_NOWAIT = 0, +} virDomainQemuAgentCommandTimeoutFlags; + # ifdef __cplusplus } # endif diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 7699042..1cfcafc 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1401,3 +1401,41 @@ qemuAgentSuspend(qemuAgentPtr mon, virJSONValueFree(reply); return ret; } + +int qemuAgentQemuAgentCommand(qemuAgentPtr mon, + const char *cmd_str, + char **result, + int timeout) +{ + int ret = -1; + int seconds; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + + cmd = virJSONValueFromString(cmd_str); + if (!cmd) + return ret; + + if (result == NULL) { + seconds = 0; + } else if (timeout == VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) { + seconds = 5; + } else { + seconds = timeout; + } + +repeat: + ret = qemuAgentCommand(mon, cmd, &reply, timeout); + + if (ret == 0) { + ret = qemuAgentCheckError(cmd, reply); + *result = virJSONValueToString(reply); + } else { + if (timeout == VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) goto repeat; + *result = NULL; + } + + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index 2fdebb2..fc19c2f 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -77,4 +77,9 @@ int qemuAgentFSThaw(qemuAgentPtr mon); int qemuAgentSuspend(qemuAgentPtr mon, unsigned int target); + +int qemuAgentQemuAgentCommand(qemuAgentPtr mon, + const char *cmd, + char **result, + int timeout); #endif /* __QEMU_AGENT_H__ */
On 08/07/2012 06:04 PM, MATSUDA, Daiki wrote:
Add qemuAgentCommand() for general qemu agent command.
include/libvirt/libvirt-qemu.h | 5 +++++ src/qemu/qemu_agent.c | 38 ++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_agent.h | 5 +++++ 3 files changed, 48 insertions(+)
Causes a compiler warning: CC libvirt_driver_qemu_impl_la-qemu_agent.lo qemu/qemu_agent.c: In function 'qemuAgentQemuAgentCommand': qemu/qemu_agent.c:1411:9: error: variable 'seconds' set but not used [-Werror=unused-but-set-variable]
diff --git a/include/libvirt/libvirt-qemu.h b/include/libvirt/libvirt-qemu.h index a37f897..ffc5ae5 100644 --- a/include/libvirt/libvirt-qemu.h +++ b/include/libvirt/libvirt-qemu.h @@ -44,6 +44,11 @@ virDomainPtr virDomainQemuAttach(virConnectPtr domain, unsigned int pid_value, unsigned int flags);
+typedef enum { + VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK = -1, + VIR_DOMAIN_QEMU_AGENT_COMMAND_NOWAIT = 0, +} virDomainQemuAgentCommandTimeoutFlags;
These aren't flags, but special timeout values. Maybe s/Flags/Values/
+ +int qemuAgentQemuAgentCommand(qemuAgentPtr mon,
This name sounds funny; maybe qemuAgentArbitraryCommand might be nicer.
+ const char *cmd_str, + char **result, + int timeout) +{ + int ret = -1; + int seconds; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + + cmd = virJSONValueFromString(cmd_str); + if (!cmd) + return ret; + + if (result == NULL) { + seconds = 0; + } else if (timeout == VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) { + seconds = 5;
Why 5 seconds here,
+ } else { + seconds = timeout;
but the user's timeout here? If you are looping while waiting for a response, then 5 seconds is too long between loops; you will feel unresponsive; and if the user passes a timeout of 10, you end up waiting 10 seconds.
+ } + +repeat: + ret = qemuAgentCommand(mon, cmd, &reply, timeout);
Won't work - this starts a new agent command every time repeat is reached, rather than waiting for the completion of an existing command.
+ + if (ret == 0) { + ret = qemuAgentCheckError(cmd, reply); + *result = virJSONValueToString(reply); + } else { + if (timeout == VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) goto repeat;
Typical formatting would be to split this to two lines: if (timeout == VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) goto repeat;
+ *result = NULL; + } + + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index 2fdebb2..fc19c2f 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -77,4 +77,9 @@ int qemuAgentFSThaw(qemuAgentPtr mon);
int qemuAgentSuspend(qemuAgentPtr mon, unsigned int target); + +int qemuAgentQemuAgentCommand(qemuAgentPtr mon, + const char *cmd, + char **result, + int timeout); #endif /* __QEMU_AGENT_H__ */
Definitely needs some work before this function will handle timeout properly. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
(2012/08/09 11:08), Eric Blake wrote:
On 08/07/2012 06:04 PM, MATSUDA, Daiki wrote:
Add qemuAgentCommand() for general qemu agent command.
include/libvirt/libvirt-qemu.h | 5 +++++ src/qemu/qemu_agent.c | 38 ++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_agent.h | 5 +++++ 3 files changed, 48 insertions(+)
Causes a compiler warning:
CC libvirt_driver_qemu_impl_la-qemu_agent.lo qemu/qemu_agent.c: In function 'qemuAgentQemuAgentCommand': qemu/qemu_agent.c:1411:9: error: variable 'seconds' set but not used [-Werror=unused-but-set-variable]
Sorry, it has a mistake on calling qemuAgentCommand() with not seconds but timeout.
diff --git a/include/libvirt/libvirt-qemu.h b/include/libvirt/libvirt-qemu.h index a37f897..ffc5ae5 100644 --- a/include/libvirt/libvirt-qemu.h +++ b/include/libvirt/libvirt-qemu.h @@ -44,6 +44,11 @@ virDomainPtr virDomainQemuAttach(virConnectPtr domain, unsigned int pid_value, unsigned int flags);
+typedef enum { + VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK = -1, + VIR_DOMAIN_QEMU_AGENT_COMMAND_NOWAIT = 0, +} virDomainQemuAgentCommandTimeoutFlags;
These aren't flags, but special timeout values. Maybe s/Flags/Values/
+ +int qemuAgentQemuAgentCommand(qemuAgentPtr mon,
This name sounds funny; maybe qemuAgentArbitraryCommand might be nicer.
+ const char *cmd_str, + char **result, + int timeout) +{ + int ret = -1; + int seconds; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + + cmd = virJSONValueFromString(cmd_str); + if (!cmd) + return ret; + + if (result == NULL) { + seconds = 0; + } else if (timeout == VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) { + seconds = 5;
Why 5 seconds here,
Becuase I realize the infinite waiting as you suggested.
* If @result is NULL, then don't wait for a result (and @timeout * must be 0). Otherwise, wait for @timeout seconds for a * successful result to be populated into *@result, with * VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK (-1) meaning to block * forever waiting for a result.
But it is difficult. So, I used 'repeat' tag and 5 seconds is default value. #define QEMU_AGENT_WAIT_TIME (1000ull * 5)
+ } else { + seconds = timeout;
but the user's timeout here? If you are looping while waiting for a response, then 5 seconds is too long between loops; you will feel unresponsive; and if the user passes a timeout of 10, you end up waiting 10 seconds.
+ } + +repeat: + ret = qemuAgentCommand(mon, cmd, &reply, timeout);
Won't work - this starts a new agent command every time repeat is reached, rather than waiting for the completion of an existing command.
+ + if (ret == 0) { + ret = qemuAgentCheckError(cmd, reply); + *result = virJSONValueToString(reply); + } else { + if (timeout == VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) goto repeat;
Typical formatting would be to split this to two lines:
if (timeout == VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) goto repeat;
+ *result = NULL; + } + + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index 2fdebb2..fc19c2f 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -77,4 +77,9 @@ int qemuAgentFSThaw(qemuAgentPtr mon);
int qemuAgentSuspend(qemuAgentPtr mon, unsigned int target); + +int qemuAgentQemuAgentCommand(qemuAgentPtr mon, + const char *cmd, + char **result, + int timeout); #endif /* __QEMU_AGENT_H__ */
Definitely needs some work before this function will handle timeout properly.
And I attached fixed patch, but it is not completed. After receiving all comments, I will resend new patches. include/libvirt/libvirt-qemu.h | 5 ++++ src/qemu/qemu_agent.c | 47 ++++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_agent.h | 5 ++++ 3 files changed, 56 insertions(+), 1 deletion(-) diff --git a/include/libvirt/libvirt-qemu.h b/include/libvirt/libvirt-qemu.h index a37f897..ffc5ae5 100644 --- a/include/libvirt/libvirt-qemu.h +++ b/include/libvirt/libvirt-qemu.h @@ -44,6 +44,11 @@ virDomainPtr virDomainQemuAttach(virConnectPtr domain, unsigned int pid_value, unsigned int flags); +typedef enum { + VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK = -1, + VIR_DOMAIN_QEMU_AGENT_COMMAND_NOWAIT = 0, +} virDomainQemuAgentCommandTimeoutValues; + # ifdef __cplusplus } # endif diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 7699042..1cfcafc 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -830,7 +830,8 @@ void qemuAgentClose(qemuAgentPtr mon) virObjectUnref(mon); } -#define QEMU_AGENT_WAIT_TIME (1000ull * 5) +#define QEMU_AGENT_WAIT_DEFAULT 5 +#define QEMU_AGENT_WAIT_TIME (1000ull * QEMU_AGENT_WAIT_DEFAULT) /** * qemuAgentSend: @@ -1401,3 +1401,47 @@ qemuAgentSuspend(qemuAgentPtr mon, virJSONValueFree(reply); return ret; } + +int qemuAgentArbitraryCommand(qemuAgentPtr mon, + const char *cmd_str, + char **result, + int timeout) +{ + int ret = -1; + int seconds; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + + if (timeout < -1) + return ret; + + cmd = virJSONValueFromString(cmd_str); + if (!cmd) + return ret; + + if (result == NULL) { + seconds = VIR_DOMAIN_QEMU_AGENT_COMMAND_NOWAIT; + } else if (timeout == VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) { + seconds = QEMU_AGENT_WAIT_DEFAULT; + } else { + seconds = timeout; + } + +repeat: + ret = qemuAgentCommand(mon, cmd, &reply, seconds); + + if (ret == 0) { + ret = qemuAgentCheckError(cmd, reply); + *result = virJSONValueToString(reply); + } else { + if (timeout == VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) { + virJSONValueFree(reply); + goto repeat; + } + *result = NULL; + } + + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index 2fdebb2..fc19c2f 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -77,4 +77,9 @@ int qemuAgentFSThaw(qemuAgentPtr mon); int qemuAgentSuspend(qemuAgentPtr mon, unsigned int target); + +int qemuAgentArbitraryCommand(qemuAgentPtr mon, + const char *cmd, + char **result, + int timeout); #endif /* __QEMU_AGENT_H__ */
participants (2)
-
Eric Blake -
MATSUDA, Daiki