Bug 807751 - [libvirt] Failed to set vm niceness with latest libvirt
[libvirt] Failed to set vm niceness with latest libvirt
Status: CLOSED ERRATA
Product: Red Hat Enterprise Linux 6
Classification: Red Hat
Component: libvirt (Show other bugs)
6.3
Unspecified Unspecified
unspecified Severity unspecified
: rc
: 6.3
Assigned To: Eric Blake
Virtualization Bugs
: Regression
Depends On: 804664
Blocks: 816609
  Show dependency treegraph
 
Reported: 2012-03-28 11:35 EDT by Eric Blake
Modified: 2012-06-20 02:51 EDT (History)
26 users (show)

See Also:
Fixed In Version: libvirt-0.9.10-10.el6
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: 804664
: 816609 (view as bug list)
Environment:
Last Closed: 2012-06-20 02:51:02 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Eric Blake 2012-03-28 11:35:25 EDT
Cloning to RHEL.

+++ This bug was initially created as a clone of Bug #804664 +++

Description of problem:

vdsm fails to set vm niceness working with latest libvirt: libvirt-0.9.10-2.fc17.x86_64

I get the following trace:

Traceback (most recent call last):
  File "/usr/share/vdsm/libvirtvm.py", line 1232, in _domDependentInit
    self._dom.setSchedulerParameters({'cpu_shares': (20 - nice) * 51})
  File "/usr/share/vdsm/libvirtvm.py", line 490, in f
    ret = attr(*args, **kwargs)
  File "/usr/share/vdsm/libvirtconnection.py", line 82, in wrapper
    ret = f(*args, **kwargs)
  File "/usr/lib64/python2.7/site-packages/libvirt.py", line 1788, in setSchedulerParameters
    ret = libvirtmod.virDomainSetSchedulerParameters(self._o, params)
SystemError: /builddir/build/BUILD/Python-2.7.2/Objects/longobject.c:980: bad argument to internal function

I suspect libvirt changed something.

note that problem doesn't occur with libvirt-0.9.9-1.fc16.x86_64.

--- Additional comment from hateya@redhat.com on 2012-03-19 08:38:25 MDT ---

The function in libvirt called: VIR_DOMAIN_SCHEDULER_CPU_SHARES, tried to look for clues in their git src with no luck, i'm sure danken would know.

--- Additional comment from hateya@redhat.com on 2012-03-19 08:45:50 MDT ---

According to eblake, libvirt team did touch the python bindings, in an effort to clean up some undefined behavior and share more code between the various clients of the C code virTypedParameter and scheduler cpu shares is a client of virTypedParameter, so maybe we introduced a regression in our typed parameter handling

--- Additional comment from eblake@redhat.com on 2012-03-28 09:33:59 MDT ---

F17 will pick up 0.9.11, where this is fixed with:

commit 1aeb3d9e7f3a967f7d5e59f852e804aef0786f6c
Author: Guannan Ren <gren@redhat.com>
Date:   Tue Mar 27 14:06:47 2012 +0800

    python: make python APIs use these helper functions
    
        *setPyVirTypedParameter
        *libvirt_virDomainGetCPUStats
Comment 5 Wayne Sun 2012-04-06 02:40:27 EDT
packages:
libvirt-0.9.10-10.el6.x86_64
libvirt-python-0.9.10-10.el6.x86_64
qemu-kvm-0.12.1.2-2.267.el6ev.x86_64
kernel-2.6.32-257.el6.x86_64

steps:
1. prepare a running domain
2. check with python
# python
Python 2.6.6 (r266:84292, Sep 12 2011, 14:03:14) 
[GCC 4.4.5 20110214 (Red Hat 4.4.5-6)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import libvirt
>>> conn = libvirt.open(None)
>>> dom = conn.lookupByName("rhel6u2")
>>> dom.setSchedulerParameters({'cpu_shares':500})
0
>>> dom.schedulerParameters()
{'vcpu_quota': -1L, 'vcpu_period': 100000L, 'cpu_shares': 500L}
>>> dom.setSchedulerParameters({'cpu_shares': 1000L})
0
>>> dom.schedulerParameters()
{'vcpu_quota': -1L, 'vcpu_period': 100000L, 'cpu_shares': 1000L}
>>> dom.setSchedulerParameters({'cpu_shares': 1000.22})
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib64/python2.6/site-packages/libvirt.py", line 1764, in setSchedulerParameters
    ret = libvirtmod.virDomainSetSchedulerParameters(self._o, params)
TypeError: an integer is required


Things also work fine with setting vcpu_period, but some odd thing happen when setting vcpu_quota.

>>> dom.setSchedulerParameters({'vcpu_quota':-1})
0
>>> dom.schedulerParameters()
{'vcpu_quota': -1L, 'vcpu_period': 100000L, 'cpu_shares': 500L}
>>> dom.setSchedulerParameters({'vcpu_quota': 0.88})
0
>>> dom.setSchedulerParameters({'vcpu_quota': 10.88})
libvir: QEMU error : Unable to set cpu bandwidth quota: Invalid argument
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib64/python2.6/site-packages/libvirt.py", line 1765, in setSchedulerParameters
    if ret == -1: raise libvirtError ('virDomainSetSchedulerParameters() failed', dom=self)
libvirt.libvirtError: Unable to set cpu bandwidth quota: Invalid argument

So PyLong_AsLongLong treat 0.88 as 0 and 10.88 as 10, maybe error should be throw out here when given float type?
Comment 6 Eric Blake 2012-04-19 17:02:58 EDT
(In reply to comment #5)
> packages:
> libvirt-0.9.10-10.el6.x86_64
> libvirt-python-0.9.10-10.el6.x86_64
> qemu-kvm-0.12.1.2-2.267.el6ev.x86_64
> kernel-2.6.32-257.el6.x86_64

> {'vcpu_quota': -1L, 'vcpu_period': 100000L, 'cpu_shares': 1000L}
> >>> dom.setSchedulerParameters({'cpu_shares': 1000.22})
> Traceback (most recent call last):
>   File "<stdin>", line 1, in <module>
>   File "/usr/lib64/python2.6/site-packages/libvirt.py", line 1764, in
> setSchedulerParameters
>     ret = libvirtmod.virDomainSetSchedulerParameters(self._o, params)
> TypeError: an integer is required
> 

> >>> dom.setSchedulerParameters({'vcpu_quota': 0.88})
> 0

> 
> So PyLong_AsLongLong treat 0.88 as 0 and 10.88 as 10, maybe error should be
> throw out here when given float type?

Yes, consistency would argue that either we always allow float-to-integer conversions (so setSchedulerParameters:cpu_shares is broken) or never allow it (so setSchedulerParameters:vcpu_quota is broken).  I'm not sure which way python tends to lean; do people like to play fast an loose with automatic floating-point-to-integer truncation, or is it better to be strict and only accept integers?
Comment 7 Gunannan Ren 2012-04-20 02:11:03 EDT
As far as I know, type-checking in python in not necessary.
If a function is designed to accept both the reference to raw string and integer object, the design is wrong and broken.
The fact is that if you don't know already the type of an object in program, then you are using the function or the program in the wrong way.

For the above example. 'vcpu_quota' need a value of PyInt_type/PyLong_type, but you give the value of Pyfloat_type, you made a mistake first.

From the code speaking, we could give a friendly type-checking, but the type-checking hurts code reuse and reduces performance commonly.
Comment 9 Eric Blake 2012-04-25 15:56:13 EDT
I'm going to split this BZ.  The original bug was that we couldn't do dom.setSchedulerParameters with reasonable (integer) arguments; this has been fixed.  The corollary issue (what do we do when setSchedulerParameters is handed unusual arguments, like floating point) is not a show-stopper to 6.3, and should be deferred to 6.4 while upstream decides how to be consistent (either to reject fractional values with a type error, or silently convert them to integers).
Comment 10 Wayne Sun 2012-05-01 22:52:25 EDT
According to comment 5 & comment 9, this bug is fixed.
Also testing with latest libvirt(libvirt-0.9.10-15.el6.x86_64) and pass.
Comment 12 errata-xmlrpc 2012-06-20 02:51:02 EDT
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHSA-2012-0748.html

Note You need to log in before you can comment on or make changes to this bug.