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