Bug 996552

Summary: Incorrect return value when setvcpus using invalid vcpu number.
Product: Red Hat Enterprise Linux 7 Reporter: Peter Krempa <pkrempa>
Component: libvirtAssignee: Peter Krempa <pkrempa>
Status: CLOSED CURRENTRELEASE QA Contact: Virtualization Bugs <virt-bugs>
Severity: medium Docs Contact:
Priority: medium    
Version: 7.0CC: acathrow, codong, dyuan, hliu, jiahu, lsu, mzhan, pkrempa
Target Milestone: rc   
Target Release: ---   
Hardware: Unspecified   
OS: Linux   
Whiteboard:
Fixed In Version: libvirt-1.1.1-3.el7 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: 996466 Environment:
Last Closed: 2014-06-13 09:29:56 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 996466    
Bug Blocks:    

Description Peter Krempa 2013-08-13 12:31:44 UTC
+++ This bug was initially created as a clone of Bug #996466 +++

Description of problem:
Incorrect return value when setvcpus using invalid vcpu number.

Version-Release number of selected component (if applicable):
libvirt-0.10.2-21.el6.x86_64
kernel-2.6.32-410.el6.x86_64
qemu-kvm-rhev-0.12.1.2-2.386.el6.x86_64

How reproducible:
always

Steps to Reproduce:
1. make sure a domain 'foo' is managed, no matter if started or shut off.
2. set vcpu number using an invalid number.
# virsh setvcpus foo --count invalid-number
3. check the return value.
# echo $?

Actual results:

output:
for step 2:
error: Invalid number of virtual CPUs
for step 3:
0

Expected results:
for step 3:
1

Additional Info:
in cmdSetvcpus of virsh-domain.c:
>   if (vshCommandOptInt(cmd, "count", &count) < 0 || count <= 0) {
>       vshError(ctl, "%s", _("Invalid number of virtual CPUs"));
>       goto cleanup;
>   }

might should be changed to:
>   if (vshCommandOptInt(cmd, "count", &count) < 0 || count <= 0) {
>       vshError(ctl, "%s", _("Invalid number of virtual CPUs"));
>       ret = false;
>       goto cleanup;
>   }

--- Additional comment from Peter Krempa on 2013-08-13 14:29:08 CEST ---

Fixed upstream with:

commit 3abb6ec077f7fdc9ac9969b5c33f2c2d270903f3
Author: Peter Krempa <pkrempa>
Date:   Tue Aug 13 11:14:56 2013 +0200

    virsh-domain: Flip logic in cmdSetvcpus
    
    To avoid having to assign a failure code to the returned variable switch
    this function to negative logic. This will fix issue with invalid number
    of cpus returning success return code.
    
    https://bugzilla.redhat.com/show_bug.cgi?id=996466

Comment 3 Hu Jianwei 2013-09-02 05:31:39 UTC
This bug can not be reproduced on libvirt-1.1.1-3.el7.x86_64.

Version:
libvirt-1.1.1-3.el7.x86_64
qemu-kvm-1.5.3-2.el7.x86_64
kernel-3.10.0-14.el7.x86_64

Steps:

1. Set a invalid value to domain.
[root@localhost ~]# virsh list --all
 Id    Name                           State
----------------------------------------------------
 -     r6                             shut off

[root@localhost ~]# virsh setvcpus r6 --count -1 --config
error: Invalid number of virtual CPUs

[root@localhost ~]# echo $?
1
[root@localhost ~]# virsh setvcpus r6 --count df --config
error: Invalid number of virtual CPUs

[root@localhost ~]# echo $?
1
[root@localhost ~]# virsh setvcpus r6 --count 3 --config
error: invalid argument: requested vcpus is greater than max allowable vcpus for the domain: 3 > 2

[root@localhost ~]# echo $?
1
[root@localhost ~]# virsh setvcpus r6 --count 0 --config
error: Invalid number of virtual CPUs

[root@localhost ~]# echo $?
1

2. Set a vaild value to domain.
[root@localhost ~]# virsh setvcpus r6 --count 1 --config

[root@localhost ~]# echo $?
0

We can get expected results, so changed to verified.

Comment 4 Ludek Smid 2014-06-13 09:29:56 UTC
This request was resolved in Red Hat Enterprise Linux 7.0.

Contact your manager or support representative in case you have further questions about the request.