Re: [libvirt] [PATCHv3 5/5] virsh: Implement command to excercise the bulk stats APIs
On 08/27/2014 12:25 PM, Peter Krempa wrote:
Add "domstats" command that excercises both of the new APIs
depending if
s/excercises/exercises/
you specify a domain list or not. The output is printed as a
key=value
list of the returned parameters.
---
tools/virsh-domain-monitor.c | 191 +++++++++++++++++++++++++++++++++++++++++++
tools/virsh.pod | 34 ++++++++
2 files changed, 225 insertions(+)
+
+static bool
+vshDomainStatsPrintRecord(vshControl *ctl ATTRIBUTE_UNUSED,
+ virDomainStatsRecordPtr record,
+ bool raw ATTRIBUTE_UNUSED)
+{
+ char *param;
+ size_t i;
+
+ vshPrint(ctl, "Domain: '%s'\n",
virDomainGetName(record->dom));
+
+ /* XXX: Implement pretty-printing */
Yeah, that can come in a later patch; this is already big enough to be a
useful first round.
+
+ VSH_EXCLUSIVE_OPTIONS("domains", "list-active");
+ VSH_EXCLUSIVE_OPTIONS("domains", "list-inactive");
+ VSH_EXCLUSIVE_OPTIONS("domains", "list-persistent");
+ VSH_EXCLUSIVE_OPTIONS("domains", "list-transient");
+ VSH_EXCLUSIVE_OPTIONS("domains", "list-running");
+ VSH_EXCLUSIVE_OPTIONS("domains", "list-paused");
+ VSH_EXCLUSIVE_OPTIONS("domains", "list-shutoff");
+ VSH_EXCLUSIVE_OPTIONS("domains", "list-other");
Drop this hunk, and then you can validate that my tweak to 4/5 actually
rejects combining things at the lower layer. Also, it is more
future-proof - if we later change our minds, and DO allow filtering over
a list of domain names, then we don't have to revisit the virsh command
to get things to work. [1]...
+=item B<domstats> [[I<--list-active>]
[I<--list-inactive>]
+[I<--list-persistent>] [I<--list-transient>] [I<--list-running>]
+[I<--list-paused>] [I<--list-shutoff>] [I<--list-other>]] |
[I<domains>]
I'd list this as [I<domains>...], to make it obvious that more than one
can be supported. Also, in most places where we have an ARGV option, we
tend to name it in the singular: echo --arg, snapshot-create-as
--diskspec, domfsfreeze --mountpoint. That has a ripple effect on the
rest of your patch s/domains/domain/
+[I<--raw>] [I<--enforce>] [I<--state>]
+
+Get statistics for multiple or all domains. Without any argument this
+command prints all available statistics for all domains.
+
+The list of domains to gather stats for can be either limited by listing
+the domains as a space separated list, or by specifying one of the
+filtering flags I<--list-*>. (The approaches can't be combined.)
...[1] But leave this comment in - we are just moving the enforcing to a
lower layer of the stack, and letting virsh expose more power so that we
have fuller testing over the lower layer behavior.
+
+ $ tools/virsh domstats --state dom1 dom2
Drop 'tools/' from the example (even if it is what you pasted from your
terminal session testing it).
+ Domain: 'dom1'
+ state.state=shut off
+ state.reason=unknown
+
+ Domain: 'dom2'
+ state.state=running
+ state.reason=booted
Doesn't match the code (yet), so how about we drop this example until we
have the followup patch for the pretty-printers.
ACK with the tweaks above.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 539 bytes)