Re: [libvirt] [PATCH] tools: fix the wrong check when use virsh setvcpus --maximum

On Fri, Mar 20, 2015 at 04:22:24PM +0800, Luyao Huang wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1204033 > > We will ignore --maximum option when only use setvcpus with > this option, like this (this error is another issue): > > # virsh setvcpus test3 --maximum 10 > error: Failed to create controller cpu for group: No such file or directory > > this is because we do not set it in flags before we check if there is > a flags set. > > Refactor these code and fix the logic. > > Signed-off-by: Luyao Huang <lhuang(a)redhat.com&gt; > --- > tools/virsh-domain.c | 33 ++++++++++++--------------------- > 1 file changed, 12 insertions(+), 21 deletions(-) > > diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c > index 1d8225c..6ab7b05 100644 > --- a/tools/virsh-domain.c > +++ b/tools/virsh-domain.c > @@ -6742,9 +6742,8 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd) > flags |= VIR_DOMAIN_AFFECT_LIVE; > if (guest) > flags |= VIR_DOMAIN_VCPU_GUEST; > - /* none of the options were specified */ > - if (!current && flags == 0) > - flags = -1; > + if (maximum) > + flags |= VIR_DOMAIN_VCPU_MAXIMUM; > > if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) > return false; > @@ -6754,27 +6753,19 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd) > goto cleanup; > } > > - if (flags == -1) { > + /* none of the options were specified */ > + if (!current && flags == 0) { > if (virDomainSetVcpus(dom, count) != 0) > goto cleanup; > } else { > - /* If the --maximum flag was given, we need to ensure only the > - --config flag is in effect as well */ > - if (maximum) { > - vshDebug(ctl, VSH_ERR_DEBUG, "--maximum flag was given\n"); > - > - flags |= VIR_DOMAIN_VCPU_MAXIMUM; > - > - /* If neither the --config nor --live flags were given, OR > - if just the --live flag was given, we need to error out > - warning the user that the --maximum flag can only be used > - with the --config flag */ > - if (live || !config) { > - > - /* Warn the user about the invalid flag combination */ > - vshError(ctl, _("--maximum must be used with --config only")); > - goto cleanup; > - } > + /* If neither the --config nor --live flags were given, OR > + if just the --live flag was given, we need to error out > + warning the user that the --maximum flag can only be used > + with the --config flag */ > + if (maximum && (live || !config)) { > + /* Warn the user about the invalid flag combination */ > + vshError(ctl, _("--maximum must be used with --config only")); > + goto cleanup; Instead of this ugly code you could've used VSH_EXCLUSIVE_OPTIONS_VAR to set that maximum and live are mutually exclusive. I've updated your patch and send it together with some other cleanups. See <https://www.redhat.com/archives/libvir-list/2015-March/msg01073.html>;.

Pavel