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@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list