On 04/15/2013 09:11 AM, Peter Krempa wrote:
This patch factors out the vCPU count retrieval including fallback means into vshCPUCountCollect() and removes the duplicated code to retrieve individual counts.
The --current flag (this flag is assumed by default) now works also with --maximum or --active without the need to explicitly specify the state of the domain that is requested.
This patch also fixes the output of "virsh vcpucount domain" on inactive domains:
Before: $ virsh vcpucount domain maximum config 4 error: Requested operation is not valid: domain is not running current config 4 error: Requested operation is not valid: domain is not running
After: $virsh vcpucount domain maximum config 4 current config 4
Looks nicer; and I don't think we ever promised that there would always be exactly four lines of output. What about the converse case, of attempting to query the config of a transient domain? Should you also clean up that output to avoid error messages?
--- tools/virsh-domain.c | 258 +++++++++++++++++++++++---------------------------- 1 file changed, 115 insertions(+), 143 deletions(-)
cmdVcpucount(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; - bool ret = true; + bool ret = false; bool maximum = vshCommandOptBool(cmd, "maximum"); bool active = vshCommandOptBool(cmd, "active"); bool config = vshCommandOptBool(cmd, "config"); bool live = vshCommandOptBool(cmd, "live"); bool current = vshCommandOptBool(cmd, "current"); bool all = maximum + active + current + config + live == 0; - int count; + unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
- /* We want one of each pair of mutually exclusive options; that - * is, use of flags requires exactly two options. We reject the - * use of more than 2 flags later on. */ - if (maximum + active + current + config + live == 1) { - if (maximum || active) { - vshError(ctl, - _("when using --%s, one of --config, --live, or --current " - "must be specified"), - maximum ? "maximum" : "active"); - } else { - vshError(ctl, - _("when using --%s, either --maximum or --active must be " - "specified"), - (current ? "current" : config ? "config" : "live")); - } - return false; - } + VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
NACK to this line, at least in this location. It prevents...
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, config); + VSH_EXCLUSIVE_OPTIONS_VAR(active, maximum);
/* Backwards compatibility: prior to 0.9.4, * VIR_DOMAIN_AFFECT_CURRENT was unsupported, and --current meant
...this backwards-compatibility hack, where '--current --live' must behave like the modern '--active --live'.
@@ -5129,140 +5196,45 @@ cmdVcpucount(vshControl *ctl, const vshCmd *cmd) active = true; }
- if (maximum && active) { - vshError(ctl, "%s", - _("--maximum and --active cannot both be specified")); - return false; - } - if (current + config + live > 1) { - vshError(ctl, "%s", - _("--config, --live, and --current are mutually exclusive")); - return false; - }
Moving it here would be okay, though. Everything else seemed okay. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org