[libvirt] [PATCH 5/5] Add qemu-agent-command command to virsh

Add qemu-agent-command command to virsh to support virDomainQemuAgentCommand(). virsh-host.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) 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 @@ -633,6 +633,74 @@ cleanup: } /* + * "qemu-agent-command" command + */ +static const vshCmdInfo info_qemu_agent_command[] = { + {"help", N_("Qemu Guest Agent Command")}, + {"desc", N_("Qemu Guest Agent Command")}, + {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")}, + {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); + } + 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; + } + + if (virDomainQemuAgentCommand(dom, guest_agent_cmd, &result, + timeout, flags) < 0) + goto cleanup; + + printf("%s\n", result); + + ret = true; + +cleanup: + VIR_FREE(result); + VIR_FREE(guest_agent_cmd); + if (dom) + virDomainFree(dom); + + return ret; +} +/* * "sysinfo" command */ static const vshCmdInfo info_sysinfo[] = { @@ -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}, {"sysinfo", cmdSysinfo, NULL, info_sysinfo, 0}, {"uri", cmdURI, NULL, info_uri, 0}, {"version", cmdVersion, opts_version, info_version, 0},

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@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake
-
MATSUDA, Daiki