[libvirt] [PATCH] tools: fix the wrong check when use virsh setvcpus --maximum
lhuang
lhuang at redhat.com
Mon Mar 23 02:06:19 UTC 2015
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
More information about the libvir-list
mailing list