Re: [libvirt] [PATCH] util: make it more robust to calculate timeout value
On 18.05.2015 03:11, Wang Yufei wrote:
> On 2015/5/15 19:16, Daniel P. Berrange wrote:
>
>> On Fri, May 15, 2015 at 01:09:09PM +0200, Michal Privoznik wrote:
>>> On 15.05.2015 08:26, zhang bo wrote:
When we change system clock to years ago, a certain CPU may use up 100%
cputime.
The reason is that in function virEventPollCalculateTimeout(), we assign
the
unsigned long long result to an INT variable,
*timeout = then - now; // timeout is INT, and then/now are long
long
if (*timeout < 0)
*timeout = 0;
there's a chance that variable @then minus variable @now may be a very
large number
that overflows INT value expression, then *timeout will be negative and be
assigned to 0.
Next the 'poll' in function virEventPollRunOnce() will get into an
'endless' while loop there.
thus, the cpu that virEventPollRunOnce() thread runs on will go up to
100%.
Although as we discussed before in
https://www.redhat.com/archives/libvir-list/2015-May/msg00400.html
it should be prohibited to set-time while other applications are running,
but it does
seems to have no harm to make the codes more robust.
Signed-off-by: Wang Yufei
Signed-off-by: Zhang Bo
---
src/util/vireventpoll.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/util/vireventpoll.c b/src/util/vireventpoll.c
index ffda206..5f5a149 100644
--- a/src/util/vireventpoll.c
+++ b/src/util/vireventpoll.c
@@ -357,9 +357,10 @@ static int virEventPollCalculateTimeout(int *timeout)
return -1;
EVENT_DEBUG("Schedule timeout then=%llu now=%llu", then, now);
-*timeout = then - now;
-if (*timeout < 0)
+if (then < now)
*timeout = 0;
+else
+*timeout = (then - now) & 0x7FFF;
>>>
>>> You're trying to make this an unsigned value. What's wrong with plain
>>> typecast?
>>>
} else {
*timeout = -1;
}
>>>
>>> I must say this is ugly. If the system clock is changed, all the
>>> timeouts should fire, shouldn't they? Otherwise important events can be
>>> missed.
>>
>> As I said in previous thread, I think that this is really just papering
>> over one specific issue, and you are still going to have a multitude of
>> problems with every app on the system when you change the system clock
>> in this kind of way. I'm just not convinced we should be trying to hack
>> around it in this one case, as it is just giving us a false illusion
>> that things are going to continue to work, when in reality they'll just
>> break somewhere else instead. eg the pthread_cond_wait() timeouts.
>>
>
>
> You're right, this patch can not fix system clock changed problem. I'm trying
> to fix the bug that assigning an unsigned long long value to an int variable,
> and
> it can fix cpu up to 100% bug. What I do is decreasing the bad effect to the
> whole
> OS. At least we can do something right.
That's why I told it's ugly. Libvirt it meant to run on many platforms,
even there where an integer is not 4 bytes long. Therefore we use plain
typecast when needed instead of masking sign bit. For instance, on
platforms where int is 2bytes, this patch will not work at all.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-php] [PATCH v1] update snapshot api
ping 2015-05-07 18:06 GMT+03:00 Vasiliy Tolstov : > 2015-05-07 17:21 GMT+03:00 Vasiliy Tolstov : >> * add constants from libvirt to snapshots api >> * add flags to snapshot functions > > This is for php libvirt binding > > > -- > Vasiliy Tolstov, > e-mail: v.tols...@selfip.ru > jabber: v...@selfip.ru -- Vasiliy Tolstov, e-mail: v.tols...@selfip.ru -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-test-api][PATCH 2/3] improve the way we get the standard deviation
On 05/18/2015 09:28 AM, Luyao Huang wrote: Signed-off-by: Luyao Huang --- utils/utils.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/utils/utils.py b/utils/utils.py index 954b2bf..a6e6965 100644 --- a/utils/utils.py +++ b/utils/utils.py @@ -906,9 +906,10 @@ def get_standard_deviation(cb1, cb2, opaque1, opaque2, number = 1000): """ D = 0 for i in range(number): -a = cb1(opaque1) +a1 = cb1(opaque1) b = cb2(opaque2) -D += (int(a) - int(b))**2 +a2 = cb1(opaque1) +D += ((int(a1) + int(a2))/2 - int(b))**2 return math.sqrt(D/number) ACK, this will spend a double time, whatever, we need a more accurate return. def param_to_tuple_nolength(paramlist): -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-test-api][PATCH 2/3] improve the way we get the standard deviation
Signed-off-by: Luyao Huang --- utils/utils.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/utils/utils.py b/utils/utils.py index 954b2bf..a6e6965 100644 --- a/utils/utils.py +++ b/utils/utils.py @@ -906,9 +906,10 @@ def get_standard_deviation(cb1, cb2, opaque1, opaque2, number = 1000): """ D = 0 for i in range(number): -a = cb1(opaque1) +a1 = cb1(opaque1) b = cb2(opaque2) -D += (int(a) - int(b))**2 +a2 = cb1(opaque1) +D += ((int(a1) + int(a2))/2 - int(b))**2 return math.sqrt(D/number) def param_to_tuple_nolength(paramlist): -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-test-api][PATCH 3/3] Add a new case for getCPUStatus
Signed-off-by: Luyao Huang
---
cases/linux_domain.conf| 6 +++
repos/domain/cpu_status.py | 113 +
2 files changed, 119 insertions(+)
create mode 100644 repos/domain/cpu_status.py
diff --git a/cases/linux_domain.conf b/cases/linux_domain.conf
index 0a7d134..9f64226 100644
--- a/cases/linux_domain.conf
+++ b/cases/linux_domain.conf
@@ -269,6 +269,12 @@ virconn:connection_security_model
guestname
$defaultname
+domain:virDomain_getCPUStats
+guestname
+$defaultname
+conn
+qemu:///system
+
domain:destroy
guestname
$defaultname
diff --git a/repos/domain/cpu_status.py b/repos/domain/cpu_status.py
new file mode 100644
index 000..6e511c0
--- /dev/null
+++ b/repos/domain/cpu_status.py
@@ -0,0 +1,113 @@
+#!/usr/bin/env python
+import libvirt
+from libvirt import libvirtError
+from utils import utils
+
+required_params = ('guestname',)
+optional_params = {'conn': 'qemu:///system'}
+
+ONLINE_CPU = '/sys/devices/system/cpu/online'
+CGROUP_PERCPU =
'/sys/fs/cgroup/cpuacct/machine.slice/machine-qemu\\x2d%s.scope/cpuacct.usage_percpu'
+CGROUP_PERVCPU =
'/sys/fs/cgroup/cpuacct/machine.slice/machine-qemu\\x2d%s.scope/vcpu%d/cpuacct.usage_percpu'
+CGROUP_USAGE =
'/sys/fs/cgroup/cpuacct/machine.slice/machine-qemu\\x2d%s.scope/cpuacct.usage'
+CGROUP_STAT =
'/sys/fs/cgroup/cpuacct/machine.slice/machine-qemu\\x2d%s.scope/cpuacct.stat'
+
+def getcputime(a):
+return open(a[0]).read().split()[a[1]]
+
+def virtgetcputime(a):
+return a[0].getCPUStats(0)[a[1]][a[2]]
+
+def getvcputime(a):
+ret = 0
+for i in range(int(a[0])):
+ret += int(open(CGROUP_PERVCPU % (a[1], i)).read().split()[a[2]])
+
+return ret
+
+def virtgettotalcputime(a):
+return a[0].getCPUStats(1)[0][a[1]]
+
+def virtgettotalcputime2(a):
+return a[0].getCPUStats(1)[0][a[1]]/1000
+
+def cpu_status(params):
+"""
+ test API for getCPUStats in class virDomain
+"""
+logger = params['logger']
+fail=0
+
+cpu = utils.file_read(ONLINE_CPU)
+logger.info("host online cpulist is %s" % cpu)
+
+cpu_tuple = utils.param_to_tuple_nolength(cpu)
+if not cpu_tuple:
+logger.info("error in function param_to_tuple_nolength")
+return 1
+
+try:
+conn = libvirt.open(params['conn'])
+
+logger.info("get connection to libvirtd")
+guest = params['guestname']
+vm = conn.lookupByName(guest)
+vcpus = vm.info()[3]
+for n in range(len(cpu_tuple)):
+if not cpu_tuple[n]:
+continue
+
+D = utils.get_standard_deviation(getcputime, virtgetcputime, \
+[CGROUP_PERCPU % guest, n], [vm,n,'cpu_time'])
+logger.info("Standard Deviation for host cpu %d cputime is %d" %
(n, D))
+
+""" expectations 403423 is a average collected in a x86_64 low
load machine"""
+if D > 403423*5:
+fail=1
+logger.info("FAIL: Standard Deviation is too big \
+ (biger than %d) for host cpu %d" % (403423*5, n))
+
+D = utils.get_standard_deviation(getvcputime, virtgetcputime, \
+[vcpus, guest, n], [vm,n,'vcpu_time'])
+logger.info("Standard Deviation for host cpu %d vcputime is %d" %
(n, D))
+
+""" expectations 4034 is a average collected in a x86_64 low load
machine"""
+if D > 4034*5*vcpus:
+fail=1
+logger.info("FAIL: Standard Deviation is too big \
+ (biger than %d) for host cpu time %d" %
(4034*5*vcpus, n))
+
+D = utils.get_standard_deviation(getcputime, virtgettotalcputime, \
+[CGROUP_USAGE % guest, 0], [vm,'cpu_time'])
+logger.info("Standard Deviation for host cpu total cputime is %d" % D)
+
+""" expectations 313451 is a average collected in a x86_64 low load
machine"""
+if D > 313451*5*len(cpu_tuple):
+fail=1
+logger.info("FAIL: Standard Deviation is too big \
+ (biger than %d) for host cpu time %d" %
(313451*5*len(cpu_tuple), n))
+
+D = utils.get_standard_deviation(getcputime, virtgettotalcputime2, \
+[CGROUP_STAT % guest, 3], [vm,'system_time'])
+logger.info("Standard Deviation for host cpu total system time is %d"
% D)
+
+""" expectations 10 is a average collected in a x86_64 low load
machine"""
+if D > 10*5:
+fail=1
+logger.info("FAIL: Standard Deviation is too big \
+ (biger than %d) for host system cpu time %d" % (10*5,
n))
+
+D = utils.get_standard_deviation(getcputime, virtgettotalcputime2, \
+[CGROUP_STAT % guest, 1], [vm,'user_time'])
+logger.info("Standard Deviation for host cpu total user time is %d" %
D)
+
+""" expectations 10 is a average collected in a x86_64 low load
m
[libvirt] [libvirt-test-api][PATCH 1/3] add new test case for getMemoryStats
Signed-off-by: Luyao Huang
---
cases/test_connection.conf | 4 ++
repos/virconn/connection_getMemoryStats.py | 96 ++
2 files changed, 100 insertions(+)
create mode 100644 repos/virconn/connection_getMemoryStats.py
diff --git a/cases/test_connection.conf b/cases/test_connection.conf
index 3c08a95..336b1ad 100644
--- a/cases/test_connection.conf
+++ b/cases/test_connection.conf
@@ -73,3 +73,7 @@ virconn:connection_getCellsFreeMemory
virconn:connection_getMemoryParameters
conn
qemu:///system
+
+virconn:connection_getMemoryStats
+conn
+qemu:///system
diff --git a/repos/virconn/connection_getMemoryStats.py
b/repos/virconn/connection_getMemoryStats.py
new file mode 100644
index 000..fcc146b
--- /dev/null
+++ b/repos/virconn/connection_getMemoryStats.py
@@ -0,0 +1,96 @@
+#!/usr/bin/env python
+import libvirt
+from libvirt import libvirtError
+from utils import utils
+
+required_params = ()
+optional_params = {'conn': ''}
+
+NODE_ONLINE = '/sys/devices/system/node/online'
+MEMINFO = '/proc/meminfo'
+
+def getsysmem(a):
+return open(a[0]).read().splitlines()[a[1]].split()[a[2]]
+
+def virtgetmem(a):
+return a[0].getMemoryStats(a[1])[a[2]]
+
+def connection_getMemoryStats(params):
+"""
+ test API for getMemoryStats in class virConnect
+"""
+logger = params['logger']
+fail=0
+
+nodeset = utils.file_read(NODE_ONLINE)
+logger.info("host exist node is %s" % nodeset)
+
+node_tuple = utils.param_to_tuple_nolength(nodeset)
+if not node_tuple:
+logger.info("error in function param_to_tuple_nolength")
+return 1
+
+try:
+conn = libvirt.open(params['conn'])
+
+logger.info("get connection cells memory status")
+for n in range(len(node_tuple)):
+if not node_tuple[n]:
+continue
+
+D = utils.get_standard_deviation(getsysmem, virtgetmem, \
+['/sys/devices/system/node/node%d/meminfo' % n,1,3],
[conn,n,'free'])
+logger.info("Standard Deviation for free memory in node %d is %d"
% (n, D))
+
+
+""" expectations 177 is a average collected in a x86_64 low load
machine"""
+if D > 177*5:
+fail=1
+logger.info("FAIL: Standard Deviation is too big \
+ (biger than %d) for node %d free memory" %
(177*5, n))
+
+a1 = ['/sys/devices/system/node/node%d/meminfo' % n, 0, 3]
+a2 = [conn,n,'total']
+if long(getsysmem(a1)) != long(virtgetmem(a2)):
+fail=1
+logger.info("FAIL: Total memory in node %d is not right" % n)
+
+
+D = utils.get_standard_deviation(getsysmem, virtgetmem, \
+[MEMINFO, 3, 1], [conn, -1, 'buffers'])
+logger.info("Standard Deviation for host buffers is %d" % D)
+
+""" expectations 30 is a average collected in a x86_64 low load
machine"""
+if D > 30*5:
+fail=1
+logger.info("FAIL: Standard Deviation is too big \
+ (biger than %d) for host buffers" % 30*5)
+
+D = utils.get_standard_deviation(getsysmem, virtgetmem, \
+[MEMINFO,4,1], [conn,-1,'cached'])
+logger.info("Standard Deviation for host cached is %d" % D)
+
+""" expectations 32 is a average collected in a x86_64 low load
machine"""
+if D > 32*5:
+fail=1
+logger.info("FAIL: Standard Deviation is too big \
+ (biger than %d) for host cached" % 32*5)
+
+D = utils.get_standard_deviation(getsysmem, virtgetmem, \
+[MEMINFO,1,1], [conn,-1,'free'])
+logger.info("Standard Deviation for host free memory is %d" % D)
+
+""" expectations 177 is a average collected in a x86_64 low load
machine"""
+if D > 177*5:
+fail=1
+logger.info("FAIL: Standard Deviation is too big \
+ (biger than %d) for host free memory" % 177*5)
+
+if long(getsysmem([MEMINFO,0,1])) !=
long(virtgetmem([conn,-1,'total'])):
+fail=1
+logger.info("FAIL: Total memory for host is not right" % n)
+
+except libvirtError, e:
+logger.error("API error message: %s" % e.message)
+fail=1
+return fail
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: make it more robust to calculate timeout value
On 2015/5/15 19:16, Daniel P. Berrange wrote:
> On Fri, May 15, 2015 at 01:09:09PM +0200, Michal Privoznik wrote:
>> On 15.05.2015 08:26, zhang bo wrote:
>>> When we change system clock to years ago, a certain CPU may use up 100%
>>> cputime.
>>> The reason is that in function virEventPollCalculateTimeout(), we assign
>>> the
>>> unsigned long long result to an INT variable,
>>> *timeout = then - now; // timeout is INT, and then/now are long long
>>> if (*timeout < 0)
>>> *timeout = 0;
>>> there's a chance that variable @then minus variable @now may be a very
>>> large number
>>> that overflows INT value expression, then *timeout will be negative and be
>>> assigned to 0.
>>> Next the 'poll' in function virEventPollRunOnce() will get into an
>>> 'endless' while loop there.
>>> thus, the cpu that virEventPollRunOnce() thread runs on will go up to 100%.
>>>
>>> Although as we discussed before in
>>> https://www.redhat.com/archives/libvir-list/2015-May/msg00400.html
>>> it should be prohibited to set-time while other applications are running,
>>> but it does
>>> seems to have no harm to make the codes more robust.
>>>
>>> Signed-off-by: Wang Yufei
>>> Signed-off-by: Zhang Bo
>>> ---
>>> src/util/vireventpoll.c | 5 +++--
>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/util/vireventpoll.c b/src/util/vireventpoll.c
>>> index ffda206..5f5a149 100644
>>> --- a/src/util/vireventpoll.c
>>> +++ b/src/util/vireventpoll.c
>>> @@ -357,9 +357,10 @@ static int virEventPollCalculateTimeout(int *timeout)
>>> return -1;
>>>
>>> EVENT_DEBUG("Schedule timeout then=%llu now=%llu", then, now);
>>> -*timeout = then - now;
>>> -if (*timeout < 0)
>>> +if (then < now)
>>> *timeout = 0;
>>> +else
>>> +*timeout = (then - now) & 0x7FFF;
>>
>> You're trying to make this an unsigned value. What's wrong with plain
>> typecast?
>>
>>> } else {
>>> *timeout = -1;
>>> }
>>>
>>
>> I must say this is ugly. If the system clock is changed, all the
>> timeouts should fire, shouldn't they? Otherwise important events can be
>> missed.
>
> As I said in previous thread, I think that this is really just papering
> over one specific issue, and you are still going to have a multitude of
> problems with every app on the system when you change the system clock
> in this kind of way. I'm just not convinced we should be trying to hack
> around it in this one case, as it is just giving us a false illusion
> that things are going to continue to work, when in reality they'll just
> break somewhere else instead. eg the pthread_cond_wait() timeouts.
>
You're right, this patch can not fix system clock changed problem. I'm trying
to fix the bug that assigning an unsigned long long value to an int variable,
and
it can fix cpu up to 100% bug. What I do is decreasing the bad effect to the
whole
OS. At least we can do something right.
>
> Regards,
> Daniel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: make it more robust to calculate timeout value
On 2015/5/15 19:09, Michal Privoznik wrote:
> On 15.05.2015 08:26, zhang bo wrote:
>> When we change system clock to years ago, a certain CPU may use up 100%
>> cputime.
>> The reason is that in function virEventPollCalculateTimeout(), we assign the
>> unsigned long long result to an INT variable,
>> *timeout = then - now; // timeout is INT, and then/now are long long
>> if (*timeout < 0)
>> *timeout = 0;
>> there's a chance that variable @then minus variable @now may be a very large
>> number
>> that overflows INT value expression, then *timeout will be negative and be
>> assigned to 0.
>> Next the 'poll' in function virEventPollRunOnce() will get into an 'endless'
>> while loop there.
>> thus, the cpu that virEventPollRunOnce() thread runs on will go up to 100%.
>>
>> Although as we discussed before in
>> https://www.redhat.com/archives/libvir-list/2015-May/msg00400.html
>> it should be prohibited to set-time while other applications are running,
>> but it does
>> seems to have no harm to make the codes more robust.
>>
>> Signed-off-by: Wang Yufei
>> Signed-off-by: Zhang Bo
>> ---
>> src/util/vireventpoll.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/util/vireventpoll.c b/src/util/vireventpoll.c
>> index ffda206..5f5a149 100644
>> --- a/src/util/vireventpoll.c
>> +++ b/src/util/vireventpoll.c
>> @@ -357,9 +357,10 @@ static int virEventPollCalculateTimeout(int *timeout)
>> return -1;
>>
>> EVENT_DEBUG("Schedule timeout then=%llu now=%llu", then, now);
>> -*timeout = then - now;
>> -if (*timeout < 0)
>> +if (then < now)
>> *timeout = 0;
>> +else
>> +*timeout = (then - now) & 0x7FFF;
>
> You're trying to make this an unsigned value. What's wrong with plain
> typecast?
No, I'm trying to give an positive value that an int variable can express,
>
>> } else {
>> *timeout = -1;
>> }
>>
>
> I must say this is ugly. If the system clock is changed, all the
> timeouts should fire, shouldn't they? Otherwise important events can be
> missed.
>
In fact, the events will not be handled.
In virEventPollDispatchTimeouts
if (eventLoop.timeouts[i].expiresAt <= (now+20)) {
...
(cb)(timer, opaque);
'expiresAt' is much bigger than 'now' because the system clock has been changed
to long long ago.
Assign an unsigned long long value to an int value is out of control, we don't
know whether it be negative
or positive because of overflow of int. I know this patch can not fix system
clock changed problem, but at
least, the 'timeout' will be under control, and the cpu up to 100% situation
can be fixed, that's my point.
> Michal
>
> .
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] C# PInvoke Library done, Nuget Package released
Libvirt C# Nuget package released at https://www.nuget.org/packages/Libvirt_Pinvoke/ This will add all necessary dll's and includes a full 1 to 1 mapping of the libvirt API 1.2.13 (http://libvirt.org/html/). All Structures, enums, delegates, and functions are included with the same naming and function. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
