Re: [libvirt] [PATCH 5/5] Add qemu-agent-command command to virsh
On 08/07/2012 06:05 PM, MATSUDA, Daiki wrote:
Add qemu-agent-command command to virsh to support
virDomainQemuAgentCommand().
virsh-host.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 70 insertions(+)
Missing documentation in virsh.pod.
diff --git a/tools/virsh-host.c b/tools/virsh-host.c
index d9d09b4..b9180f3 100644
--- a/tools/virsh-host.c
+++ b/tools/virsh-host.c
Odd (I would have expected virsh-domain.c, since this is a command
related to an online domain), but pre-existing (qemu-monitor-command is
also here, so your patch did the right thing in being in the same location).
@@ -633,6 +633,74 @@ cleanup:
}
/*
+ * "qemu-agent-command" command
+ */
+static const vshCmdInfo info_qemu_agent_command[] = {
+ {"help", N_("Qemu Guest Agent Command")},
s/Qemu/QEMU/ or the qemu folks will be on our case for misspelling it
(besides, you should match the style of qemu-monitor-command).
+ {"desc", N_("Qemu Guest Agent Command")},
Here, you are copying from a poor example (so fix qemu-monitor-command
in the meantime); I'd suggest:
Run an arbitrary qemu guest agent command; use at your own risk
and yes, I really do suggest the 'at your own risk' phrase, since we
provide this strictly as a debugging aid rather than a supported API.
Which means qemu-monitor-command desc should list:
Run an arbitrary qemu monitor command; use at your own risk
+ {NULL, NULL}
+};
+
+static const vshCmdOptDef opts_qemu_agent_command[] = {
+ {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or
uuid")},
+ {"timeout", VSH_OT_INT, VSH_OFLAG_REQ_OPT, N_("timeout")},
+ {"cmd", VSH_OT_ARGV, VSH_OFLAG_REQ, N_("command")},
When the user does not specify --timeout, do we normally want to provide
a decent default timeout, or do we want to issue a non-blocking call
where we don't wait for an answer? That almost argues that we need yet
another bool option that lets the user choose to be --async (no waiting)
vs omitted (block for an answer, and missing --timeout implies a sane
default, such as 5 seconds, rather than 0).
+ {NULL, 0, 0, NULL}
+};
+
+static bool
+cmdQemuAgentCommand(vshControl *ctl, const vshCmd *cmd)
+{
+ virDomainPtr dom = NULL;
+ bool ret = false;
+ char *guest_agent_cmd = NULL;
+ char *result = NULL;
+ int timeout = 0;
+ unsigned int flags = 0;
+ const vshCmdOpt *opt = NULL;
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+ bool pad = false;
+
+ if (!vshConnectionUsability(ctl, ctl->conn))
+ goto cleanup;
+
+ dom = vshCommandOptDomain(ctl, cmd, NULL);
+ if (dom == NULL)
+ goto cleanup;
+
+ while ((opt = vshCommandOptArgv(cmd, opt))) {
+ if (pad)
+ virBufferAddChar(&buf, ' ');
+ pad = true;
+ virBufferAdd(&buf, opt->data, -1);
+ }
This mess with pad comes because qemu-monitor-command was written before
virBufferTrim. I'd simplify this to:
while ((opt = vshCommandOptArgv(cmd, opt)))
virBufferAsprintf(&buf, "%s ", opt->data)
virBufferTrim(&buf, " ", 1);
+ if (virBufferError(&buf)) {
+ vshPrint(ctl, "%s", _("Failed to collect command"));
+ goto cleanup;
+ }
+ guest_agent_cmd = virBufferContentAndReset(&buf);
+
+ if (vshCommandOptInt(cmd, "timeout", &timeout) < 0) {
+ timeout = 0;
Wrong. If vshCommandOptInt is < 0, you need to fail, because the user
had a syntax error on the command line. If it is equal to 0, then
timeout is already 0 (because you initialized it earlier); but I was
arguing that you should have pre-initialized to 5 instead of 0.
+ }
+
+ if (virDomainQemuAgentCommand(dom, guest_agent_cmd, &result,
+ timeout, flags) < 0)
Again, you need to handle the case where the user doesn't want to wait
for output, so you need to be able to pass in NULL instead of &result.
@@ -832,6 +900,8 @@ static const vshCmdDef hostAndHypervisorCmds[] =
{
{"qemu-attach", cmdQemuAttach, opts_qemu_attach, info_qemu_attach, 0},
{"qemu-monitor-command", cmdQemuMonitorCommand,
opts_qemu_monitor_command,
info_qemu_monitor_command, 0},
+ {"qemu-agent-command", cmdQemuAgentCommand, opts_qemu_agent_command,
+ info_qemu_agent_command, 0},
Sorting is off.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 620 bytes)