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