On 08/07/2012 06:05 PM, MATSUDA, Daiki wrote:
Add virDomainQemuAgentCommand() to qemu driver and remote driver to support qemuAgentCommand().
daemon/remote.c | 35 ++++++++++++++++++++ include/libvirt/libvirt-qemu.h | 3 + src/driver.h | 6 +++ src/libvirt-qemu.c | 58 +++++++++++++++++++++++++++++++++ src/libvirt_qemu.syms | 5 ++ src/qemu/qemu_driver.c | 71 +++++++++++++++++++++++++++++++++++++++++ src/qemu_protocol-structs | 10 +++++ src/remote/qemu_protocol.x | 14 +++++++- src/remote/remote_driver.c | 41 +++++++++++++++++++++++ 9 files changed, 242 insertions(+), 1 deletion(-)
You are mixing the addition of new API (include/, src/driver.h, src/libvirt-qemu.{c,syms}) with two implementations of two drivers (daemon and src/remote for the RPC, and src/qemu for the qemu implementation). If this were an API in libvirt.h, I'd split this to three patches, but since it is for libvirt-qemu.h, I'm 50-50 whether the split makes sense. Compilation failure; you need to rebase to latest libvirt.git: CC libvirt_driver_qemu_impl_la-qemu_driver.lo qemu/qemu_driver.c: In function 'qemuDomainQemuAgentCommand': qemu/qemu_driver.c:13228:9: error: implicit declaration of function 'qemuReportError' [-Werror=implicit-function-declaration] qemu/qemu_driver.c:13228:9: error: nested extern declaration of 'qemuReportError' [-Werror=nested-externs]
diff --git a/daemon/remote.c b/daemon/remote.c index 832307e..4fd5c9b 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -3948,6 +3948,41 @@ cleanup: return rv; }
+static int +qemuDispatchAgentCommand(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + qemu_agent_command_args *args, + qemu_agent_command_ret *ret)
Indentation is off. This function looks pretty simple; any reason we can't autogenerate it from the .x file?
@@ -1048,6 +1053,7 @@ struct _virDriver { virDrvDomainGetDiskErrors domainGetDiskErrors; virDrvDomainSetMetadata domainSetMetadata; virDrvDomainGetMetadata domainGetMetadata; + virDrvDomainQemuAgentCommand qemuDomainQemuAgentCommand;
I'd group this line next to the other virDomainQemu callback.
+++ b/src/libvirt-qemu.c @@ -185,3 +185,61 @@ error: virDispatchError(conn); return NULL; } + +/** + * virDomainQemuAgentCommand: + * @domain: a domain object + * @cmd: the guest agent command string + * @result: a string returned by @cmd + * @timeout: timeout seconds + * @flags: execution flags + * + * Execution Guest Agent Command
Execute an arbitrary Guest Agent command.
+ * + * Issue @cmd to the guest agent running in @domain. + * 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. + * + * Returns 0 if succeeded, -1 in failing.
s/if succeeded/on success/; s/in failing/on failure/
+ */ +int +virDomainQemuAgentCommand(virDomainPtr domain, + const char *cmd, + char **result, + int timeout, + unsigned int flags) +{ + virConnectPtr conn;
+ conn = domain->conn; + + virCheckNonNullArgGoto(result, error);
Wrong - we allow a NULL result argument, to issue a command with no expected response.
@@ -13369,6 +13439,7 @@ static virDriver qemuDriver = { .domainPMSuspendForDuration = qemuDomainPMSuspendForDuration, /* 0.9.11 */ .domainPMWakeup = qemuDomainPMWakeup, /* 0.9.11 */ .domainGetCPUStats = qemuDomainGetCPUStats, /* 0.9.11 */ + .qemuDomainQemuAgentCommand = qemuDomainQemuAgentCommand, /* 0.10.0 */
Again, sticking this next to the other qemuDomainQemu callback would be nicer.
+++ b/src/remote/qemu_protocol.x @@ -47,6 +47,17 @@ struct qemu_domain_attach_ret { remote_nonnull_domain dom; };
+struct qemu_agent_command_args { + remote_nonnull_domain dom; + remote_nonnull_string cmd; + int timeout; + unsigned int flags;
Missing a field to pass to the other end of the connection whether the result pointer was given as NULL.
+}; + +struct qemu_agent_command_ret { + remote_nonnull_string result;
If the result pointer was NULL, then there is no result to return; I think this has to be remote_string.
+}; + /* Define the program number, protocol version and procedure numbers here. */ const QEMU_PROGRAM = 0x20008087; const QEMU_PROTOCOL_VERSION = 1; @@ -61,5 +72,6 @@ enum qemu_procedure { * are some exceptions to this rule, e.g. domainDestroy. Other APIs MAY * be marked as high priority. If in doubt, it's safe to choose low. */ QEMU_PROC_MONITOR_COMMAND = 1, /* skipgen skipgen priority:low */ - QEMU_PROC_DOMAIN_ATTACH = 2 /* autogen autogen priority:low */ + QEMU_PROC_DOMAIN_ATTACH = 2, /* autogen autogen priority:low */ + QEMU_PROC_AGENT_COMMAND = 3 /* skipgen skipgen priority:low */
Again, can you use autogen instead of skipgen?
+++ b/src/remote/remote_driver.c @@ -5191,6 +5191,46 @@ make_nonnull_domain_snapshot (remote_nonnull_domain_snapshot *snapshot_dst, virD make_nonnull_domain(&snapshot_dst->dom, snapshot_src->domain); }
+static int +remoteQemuDomainQemuAgentCommand (virDomainPtr domain, const char *cmd, + char **result, int timeout, + unsigned int flags) +{ + int rv = -1; + qemu_agent_command_args args; + qemu_agent_command_ret ret; + struct private_data *priv = domain->conn->privateData; + + remoteDriverLock(priv); + + make_nonnull_domain(&args.dom, domain); + args.cmd = (char *)cmd; + args.timeout = timeout; + args.flags = flags; + + memset (&ret, 0, sizeof(ret)); + + if (call (domain->conn, priv, REMOTE_CALL_QEMU, QEMU_PROC_AGENT_COMMAND, + (xdrproc_t) xdr_qemu_agent_command_args, (char *) &args, + (xdrproc_t) xdr_qemu_agent_command_ret, (char *) &ret) == -1) + goto done; + + *result = strdup(ret.result);
You need to account for the case when result is NULL (maybe you can't autogen after all, if the generator can't handle an optionally NULL argument). -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org