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

[lhuang]

On 03/20/2015 11:01 PM, Pavel Hrdina wrote:
> 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 at redhat.com>
>> ---
>>   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>.

Okay, i have saw your patches, it is a good idea and will make codes 
clean, thanks for your review.

> Pavel
>
>

Luyao