Roman Mohr has uploaded a new change for review.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
host stats: Collect stats from online cpu cores only
When a cpu goes offline while vdsm is running, the getVdsStats call still tries to collect sampling data for this CPU and fails with an exception.
When vdsm is already running, it is enough to do something like
echo 0 > /sys/devices/system/cpu/cpu2/online
to break getVdsStats.
Change-Id: Ia9c247f9138e02a9230a0849a04cb2e1705e7fac Signed-off-by: Roman Mohr rmohr@redhat.com --- M tests/samplingTests.py M tests/vmfakelib.py M vdsm/virt/sampling.py 3 files changed, 47 insertions(+), 16 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/69/46269/4
diff --git a/tests/samplingTests.py b/tests/samplingTests.py index 73e88b1..49b0410 100644 --- a/tests/samplingTests.py +++ b/tests/samplingTests.py @@ -313,34 +313,61 @@ self.assertEqual(last.id, FakeHostSample.counter - 1)
- def testCpuCoreStats(self): - node_id, cpu_id = 0, 0 - cpu_sample = {'user': 1.0, 'sys': 2.0} - - first_sample = fake.HostSample(1.0, {cpu_id: cpu_sample}) - last_sample = fake.HostSample(2.0, {cpu_id: cpu_sample}) - + def _monkeyPatchedFakeNumaTopology(self): def fakeNumaTopology(): return { - node_id: { - 'cpus': [cpu_id] + 0: { + 'cpus': [0] + }, + 1: { + 'cpus': [1] } } + return MonkeyPatchScope([(caps, 'getNumaTopology', + fakeNumaTopology)])
- expected = { - '0': { + def _expectedResult(self, node_index=0): + return { + str(node_index): { 'cpuIdle': '100.00', 'cpuSys': '0.00', 'cpuUser': '0.00', - 'nodeIndex': 0 + 'nodeIndex': node_index } }
- with MonkeyPatchScope([(caps, 'getNumaTopology', - fakeNumaTopology)]): + def testCpuCoreStats(self): + cpu_sample = {'user': 1.0, 'sys': 2.0} + + first_sample = fake.HostSample(1.0, {0: cpu_sample, 1: cpu_sample}) + last_sample = fake.HostSample(2.0, {0: cpu_sample, 1: cpu_sample}) + + with self._monkeyPatchedFakeNumaTopology(): + result = sampling._get_cpu_core_stats(first_sample, last_sample) + self.assertEqual(result['0'], self._expectedResult()['0']) + self.assertEqual(result['1'], self._expectedResult(1)['1']) + + def testSkipStatsOnMissingFirstSample(self): + cpu_sample = {'user': 1.0, 'sys': 2.0} + + first_sample = fake.HostSample(1.0, {0: cpu_sample}) + last_sample = fake.HostSample(2.0, {0: cpu_sample, 1: cpu_sample}) + + with self._monkeyPatchedFakeNumaTopology(): self.assertEqual( sampling._get_cpu_core_stats(first_sample, last_sample), - expected) + self._expectedResult()) + + def testSkipStatsOnMissingLastSample(self): + cpu_sample = {'user': 1.0, 'sys': 2.0} + + first_sample = fake.HostSample(1.0, {0: cpu_sample, 1: cpu_sample}) + last_sample = fake.HostSample(2.0, {0: cpu_sample}) + + with self._monkeyPatchedFakeNumaTopology(): + self.assertEqual( + sampling._get_cpu_core_stats(first_sample, last_sample), + self._expectedResult())
class NumaNodeMemorySampleTests(TestCaseBase): diff --git a/tests/vmfakelib.py b/tests/vmfakelib.py index 99cbcb9..361919d 100644 --- a/tests/vmfakelib.py +++ b/tests/vmfakelib.py @@ -383,7 +383,7 @@ self._samples = samples
def getCoreSample(self, key): - return self._samples[key] + return self._samples.get(key)
class HostSample(object): diff --git a/vdsm/virt/sampling.py b/vdsm/virt/sampling.py index 1ad9d46..b6b5bc9 100644 --- a/vdsm/virt/sampling.py +++ b/vdsm/virt/sampling.py @@ -691,6 +691,10 @@ for node_index, numa_node in six.iteritems(caps.getNumaTopology()): cpu_cores = numa_node['cpus'] for cpu_core in cpu_cores: + # Do not try to collect cpu core data when no samples are present + if (not last_sample.cpuCores.getCoreSample(cpu_core) or not + first_sample.cpuCores.getCoreSample(cpu_core)): + continue core_stat = { 'nodeIndex': int(node_index), 'cpuUser': compute_cpu_usage(cpu_core, 'user'),
automation@ovirt.org has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 1:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Roman Mohr has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 1: Verified+1
automation@ovirt.org has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 2:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
automation@ovirt.org has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 3:
* Update tracker::#1264003::OK * Check Bug-Url::OK * Check Public Bug::#1264003::OK, public bug * Check Product::#1264003::SKIPPED, not (oVirt Red Hat Enterprise Virtualization Manager) product but vdsm * Check Product::WARN, no bug url with correct product found, make sure you have at least one bug-url with a product in oVirt Red Hat Enterprise Virtualization Manager. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Francesco Romani has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 3: Code-Review+1
makes sense. Please check also that the schema allows us to skip this field (it should)
Francesco Romani has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 3:
please add one unit test or one functional test
automation@ovirt.org has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 4:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Roman Mohr has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 4: Verified+1
Roman Mohr has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 4:
please add one unit test or one functional test
Done
Roman Mohr has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 4:
makes sense. Please check also that the schema allows us to skip this field (it should)
Regarding to the schema the cpu_core_stats map has to be there but it can be empty
Francesco Romani has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 4: Code-Review-1
(2 comments)
let's try to simplify the tests. Codewise looks good.
https://gerrit.ovirt.org/#/c/46269/4/tests/samplingTests.py File tests/samplingTests.py:
Line 323: 'cpus': [1] Line 324: } Line 325: } Line 326: return MonkeyPatchScope([(caps, 'getNumaTopology', Line 327: fakeNumaTopology)]) unless you have a clear case for this (please elaborate), I'd like to: - have fakeNumaTopology as module level function, or maybe added to vmfakelib.py - use MonkeyPatchScope inline, getting rid of this helper. Line 328: Line 329: def _expectedResult(self, node_index=0): Line 330: return { Line 331: str(node_index): {
Line 333: 'cpuSys': '0.00', Line 334: 'cpuUser': '0.00', Line 335: 'nodeIndex': node_index Line 336: } Line 337: } same, please inline Line 338: Line 339: def testCpuCoreStats(self): Line 340: cpu_sample = {'user': 1.0, 'sys': 2.0} Line 341:
Nir Soffer has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 4:
(1 comment)
https://gerrit.ovirt.org/#/c/46269/4/vdsm/virt/sampling.py File vdsm/virt/sampling.py:
Line 692: cpu_cores = numa_node['cpus'] Line 693: for cpu_core in cpu_cores: Line 694: # Do not try to collect cpu core data when no samples are present Line 695: if (not last_sample.cpuCores.getCoreSample(cpu_core) or not Line 696: first_sample.cpuCores.getCoreSample(cpu_core)): Why not fix compute_cpu_usage to return -1 or 0 for an offline core?
Do we prefer no entry for the offline cpu in the stats? Line 697: continue Line 698: core_stat = { Line 699: 'nodeIndex': int(node_index), Line 700: 'cpuUser': compute_cpu_usage(cpu_core, 'user'),
Roman Mohr has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 4:
(1 comment)
https://gerrit.ovirt.org/#/c/46269/4/vdsm/virt/sampling.py File vdsm/virt/sampling.py:
Line 692: cpu_cores = numa_node['cpus'] Line 693: for cpu_core in cpu_cores: Line 694: # Do not try to collect cpu core data when no samples are present Line 695: if (not last_sample.cpuCores.getCoreSample(cpu_core) or not Line 696: first_sample.cpuCores.getCoreSample(cpu_core)):
Why not fix compute_cpu_usage to return -1 or 0 for an offline core?
Fixing compute_cpu_usage is fine for me too.
Returning nothing for an offline core, is what we already do when the core was already offline when vdsm started. Line 697: continue Line 698: core_stat = { Line 699: 'nodeIndex': int(node_index), Line 700: 'cpuUser': compute_cpu_usage(cpu_core, 'user'),
Nir Soffer has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 4:
(1 comment)
https://gerrit.ovirt.org/#/c/46269/4/vdsm/virt/sampling.py File vdsm/virt/sampling.py:
Line 692: cpu_cores = numa_node['cpus'] Line 693: for cpu_core in cpu_cores: Line 694: # Do not try to collect cpu core data when no samples are present Line 695: if (not last_sample.cpuCores.getCoreSample(cpu_core) or not Line 696: first_sample.cpuCores.getCoreSample(cpu_core)):
Fixing compute_cpu_usage is fine for me too.
By returning nothing, you mean there is no entry for this cpu in the returned map? Line 697: continue Line 698: core_stat = { Line 699: 'nodeIndex': int(node_index), Line 700: 'cpuUser': compute_cpu_usage(cpu_core, 'user'),
Nir Soffer has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 4:
(1 comment)
https://gerrit.ovirt.org/#/c/46269/4//COMMIT_MSG Commit Message:
Line 12: When vdsm is already running, it is enough to do something like Line 13: Line 14: echo 0 > /sys/devices/system/cpu/cpu2/online Line 15: Line 16: to break getVdsStats. To make your change more clear, please show a fraction of the stats vdsm returns after your patch.
Also describe how offline cpus are handled - both those that were offline where vdsm started, and those that became offline while vdsm was running.
And what about cpu that was offline when vdsm started, and became online while vdsm was running? Line 17: Line 18: Change-Id: Ia9c247f9138e02a9230a0849a04cb2e1705e7fac
automation@ovirt.org has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 5:
* Update tracker::#1264003::OK * Check Bug-Url::OK * Check Public Bug::#1264003::OK, public bug * Check Product::#1264003::SKIPPED, not (oVirt Red Hat Enterprise Virtualization Manager) product but vdsm * Check Product::WARN, no bug url with correct product found, make sure you have at least one bug-url with a product in oVirt Red Hat Enterprise Virtualization Manager. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Roman Mohr has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 4:
(2 comments)
https://gerrit.ovirt.org/#/c/46269/4//COMMIT_MSG Commit Message:
Line 12: When vdsm is already running, it is enough to do something like Line 13: Line 14: echo 0 > /sys/devices/system/cpu/cpu2/online Line 15: Line 16: to break getVdsStats.
To make your change more clear, please show a fraction of the stats vdsm re
For now added to https://bugzilla.redhat.com/show_bug.cgi?id=1264003. Turns out to be more complicated why things are like they are. Line 17: Line 18: Change-Id: Ia9c247f9138e02a9230a0849a04cb2e1705e7fac
https://gerrit.ovirt.org/#/c/46269/4/vdsm/virt/sampling.py File vdsm/virt/sampling.py:
Line 692: cpu_cores = numa_node['cpus'] Line 693: for cpu_core in cpu_cores: Line 694: # Do not try to collect cpu core data when no samples are present Line 695: if (not last_sample.cpuCores.getCoreSample(cpu_core) or not Line 696: first_sample.cpuCores.getCoreSample(cpu_core)):
By returning nothing, you mean there is no entry for this cpu in the return
Turned out that it depends on libvirtd if we see the exception or no report for the offline cpu.
When there are three cpus (0,1,2) and cpu 1 is offline when libvirtd is started, we only show sampling data for cpu 0 and 2. When the cpu is disabled while libvirtd is running, we run into the exception.
So maybe we should not only fix compute_cpu_usage, but also getNumaTopology() that it will always behave the same way, no matter when the cpu went offline.
Or we can leave getNumaTopology() as it is and use _get_online_cpus instead, wich really just reports online cpus.
But still I don't know if we should report nothing at all. But I think when we skip the entry, we should add cpuCores, cpuThreads and cpuSockets along with onlineCpus too to getVdsStats, because they also change when the cpu goes offline
The exact behaviour is documentend in a document attached to the bug report Line 697: continue Line 698: core_stat = { Line 699: 'nodeIndex': int(node_index), Line 700: 'cpuUser': compute_cpu_usage(cpu_core, 'user'),
Francesco Romani has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 4:
(1 comment)
https://gerrit.ovirt.org/#/c/46269/4/vdsm/virt/sampling.py File vdsm/virt/sampling.py:
Line 692: cpu_cores = numa_node['cpus'] Line 693: for cpu_core in cpu_cores: Line 694: # Do not try to collect cpu core data when no samples are present Line 695: if (not last_sample.cpuCores.getCoreSample(cpu_core) or not Line 696: first_sample.cpuCores.getCoreSample(cpu_core)):
Turned out that it depends on libvirtd if we see the exception or no report
we should be able to fix this using online_cpus() you are adding in the other patch. Please check my comment in
https://gerrit.ovirt.org/#/c/46270/4/vdsm/virt/sampling.py Line 697: continue Line 698: core_stat = { Line 699: 'nodeIndex': int(node_index), Line 700: 'cpuUser': compute_cpu_usage(cpu_core, 'user'),
automation@ovirt.org has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 6:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Roman Mohr has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 4:
(5 comments)
https://gerrit.ovirt.org/#/c/46269/4//COMMIT_MSG Commit Message:
Line 12: When vdsm is already running, it is enough to do something like Line 13: Line 14: echo 0 > /sys/devices/system/cpu/cpu2/online Line 15: Line 16: to break getVdsStats.
To make your change more clear, please show a fraction of the stats vdsm re
Done Line 17: Line 18: Change-Id: Ia9c247f9138e02a9230a0849a04cb2e1705e7fac
https://gerrit.ovirt.org/#/c/46269/4/tests/samplingTests.py File tests/samplingTests.py:
Line 323: 'cpus': [1] Line 324: } Line 325: } Line 326: return MonkeyPatchScope([(caps, 'getNumaTopology', Line 327: fakeNumaTopology)])
unless you have a clear case for this (please elaborate), I'd like to:
* use MonkeyPatchScope inline: done * We have different tests which require different numa topologies, so it seems like an obvious choice for me to keep it in the class Line 328: Line 329: def _expectedResult(self, node_index=0): Line 330: return { Line 331: str(node_index): {
Line 333: 'cpuSys': '0.00', Line 334: 'cpuUser': '0.00', Line 335: 'nodeIndex': node_index Line 336: } Line 337: }
same, please inline
Not exactly sure how this should look like.
This expectedResult is used in four different tests. Do you mean that I should add the corresponding expanded map to every test? Line 338: Line 339: def testCpuCoreStats(self): Line 340: cpu_sample = {'user': 1.0, 'sys': 2.0} Line 341:
https://gerrit.ovirt.org/#/c/46269/4/vdsm/virt/sampling.py File vdsm/virt/sampling.py:
Line 692: cpu_cores = numa_node['cpus'] Line 693: for cpu_core in cpu_cores: Line 694: # Do not try to collect cpu core data when no samples are present Line 695: if (not last_sample.cpuCores.getCoreSample(cpu_core) or not Line 696: first_sample.cpuCores.getCoreSample(cpu_core)):
we should be able to fix this using online_cpus() you are adding in the oth
We can't just filter offline cpus. When a CPU suddenly reappears, it does not necessarily have the two required samples. When checking if both samples are already there we are on the save side.
When the CPU is online again for some time all the samples are there and the stats will be reported Line 697: continue Line 698: core_stat = { Line 699: 'nodeIndex': int(node_index), Line 700: 'cpuUser': compute_cpu_usage(cpu_core, 'user'),
Line 692: cpu_cores = numa_node['cpus'] Line 693: for cpu_core in cpu_cores: Line 694: # Do not try to collect cpu core data when no samples are present Line 695: if (not last_sample.cpuCores.getCoreSample(cpu_core) or not Line 696: first_sample.cpuCores.getCoreSample(cpu_core)):
Why not fix compute_cpu_usage to return -1 or 0 for an offline core?
I have been thinking again about fixing compute_cpu_usage and I think it is better to leave it this way. compute_cpu_usage is already a local function and the way it is written makes the data population process very readable.
As long as it is ok to just skip offline CPUs (and I think it is, because then we have a consistent behaviour), I think it is more readable. Line 697: continue Line 698: core_stat = { Line 699: 'nodeIndex': int(node_index), Line 700: 'cpuUser': compute_cpu_usage(cpu_core, 'user'),
Roman Mohr has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 6: Verified+1
Francesco Romani has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 6:
(3 comments)
https://gerrit.ovirt.org/#/c/46269/6//COMMIT_MSG Commit Message:
Line 29: 'cpuUser': '3.46', Line 30: 'nodeIndex': 0}} Line 31: [...] Line 32: Line 33: When only one CPU out of two has been online before libvird was typo: libvirtd Line 34: started, vdsm reports: Line 35: Line 36: $ vdsClient -s 0 getVdsStats Line 37: [...]
Line 40: 'cpuUser': '7.86', Line 41: 'nodeIndex': 0}} Line 42: [...] Line 43: Line 44: When one CPU went offline after libvird has been started, vdsm threw typo: libvirtd Line 45: an exception. With this patch getVdsStats reports only stats from the Line 46: remaining online CPU: Line 47: Line 48: $ vdsClient -s 0 getVdsStats
https://gerrit.ovirt.org/#/c/46269/6/vdsm/virt/sampling.py File vdsm/virt/sampling.py:
Line 693: for cpu_core in cpu_cores: Line 694: # Do not try to collect cpu core data when no samples are present Line 695: if (not last_sample.cpuCores.getCoreSample(cpu_core) or not Line 696: first_sample.cpuCores.getCoreSample(cpu_core)): Line 697: continue This works, but I'm not sure we want to do this instead of using a try/except block around core_stat handling below.
Something like:
try: core_stat = { 'nodeIndex': int(node_index), 'cpuUser': compute_cpu_usage(cpu_core, 'user'), 'cpuSys': compute_cpu_usage(cpu_core, 'sys'), } except KeyError: # cpu onlined or offlined recently, not enough samples yet pass else: core_stat['cpuIdle'] = ( "%.2f" % max(0.0, 100.0 - float(core_stat['cpuUser']) - float(core_stat['cpuSys']))) cpu_core_stats[str(cpu_core)] = core_stat Line 698: core_stat = { Line 699: 'nodeIndex': int(node_index), Line 700: 'cpuUser': compute_cpu_usage(cpu_core, 'user'), Line 701: 'cpuSys': compute_cpu_usage(cpu_core, 'sys'),
Nir Soffer has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 6:
(2 comments)
https://gerrit.ovirt.org/#/c/46269/6//COMMIT_MSG Commit Message:
Line 26: 'nodeIndex': 0}, Line 27: '1': {'cpuIdle': '96.07', Line 28: 'cpuSys': '0.47', Line 29: 'cpuUser': '3.46', Line 30: 'nodeIndex': 0}} The indentation make it hard to understand this format - it is
{ '0': {...}, '1': {...}, ... }
Right?
Then, regarding https://gerrit.ovirt.org/46270, why not add the status of the cpu in the same dict?
For example - online cpu:
{ 'online': True, cpuIdle: 90.74, 'cpuSys': '1.40', 'cpuUser': '7.86', 'nodeIndex': 0 }
And offline:
{ 'online': False, 'nodeIndex': 0 }
Having all the information in one data structure is nicer to the consumer of this api. Line 31: [...] Line 32: Line 33: When only one CPU out of two has been online before libvird was Line 34: started, vdsm reports:
Line 50: cpuStatistics = {'0': {'cpuIdle': '90.74', Line 51: 'cpuSys': '1.40', Line 52: 'cpuUser': '7.86', Line 53: 'nodeIndex': 0}} Line 54: [...] We should report all cpus, with the relevant info for each.
We should have the same format regardless cpus taken offline or online while libvirtd/vdsm are running. Line 55: Line 56: Change-Id: Ia9c247f9138e02a9230a0849a04cb2e1705e7fac
Roman Mohr has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 6:
(1 comment)
https://gerrit.ovirt.org/#/c/46269/6//COMMIT_MSG Commit Message:
Line 26: 'nodeIndex': 0}, Line 27: '1': {'cpuIdle': '96.07', Line 28: 'cpuSys': '0.47', Line 29: 'cpuUser': '3.46', Line 30: 'nodeIndex': 0}}
The indentation make it hard to understand this format - it is
This part is about statistics and not about the status. So I am undecided if this is really better.
Anyway I first have to check if the engine could handle that. Line 31: [...] Line 32: Line 33: When only one CPU out of two has been online before libvird was Line 34: started, vdsm reports:
Roman Mohr has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 6:
(1 comment)
https://gerrit.ovirt.org/#/c/46269/6/vdsm/virt/sampling.py File vdsm/virt/sampling.py:
Line 693: for cpu_core in cpu_cores: Line 694: # Do not try to collect cpu core data when no samples are present Line 695: if (not last_sample.cpuCores.getCoreSample(cpu_core) or not Line 696: first_sample.cpuCores.getCoreSample(cpu_core)): Line 697: continue
This works, but I'm not sure we want to do this instead of using a try/exce
It is perfectly fine to not have all samples when cpus change their state, so I would prefer to not use exceptions for that. Line 698: core_stat = { Line 699: 'nodeIndex': int(node_index), Line 700: 'cpuUser': compute_cpu_usage(cpu_core, 'user'), Line 701: 'cpuSys': compute_cpu_usage(cpu_core, 'sys'),
Francesco Romani has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 6:
(1 comment)
https://gerrit.ovirt.org/#/c/46269/6/vdsm/virt/sampling.py File vdsm/virt/sampling.py:
Line 693: for cpu_core in cpu_cores: Line 694: # Do not try to collect cpu core data when no samples are present Line 695: if (not last_sample.cpuCores.getCoreSample(cpu_core) or not Line 696: first_sample.cpuCores.getCoreSample(cpu_core)): Line 697: continue
It is perfectly fine to not have all samples when cpus change their state,
Yes, but I'm under the impression that we are supposed to have samples for all the cpus all the times (of course at steady state). Or, to say it differently, that offlining/onlining a CPU while VDSM runs is "exceptional". Right?
Besides that, why you'd like to avoid exceptions? code style, code clarity, performance, something else? Line 698: core_stat = { Line 699: 'nodeIndex': int(node_index), Line 700: 'cpuUser': compute_cpu_usage(cpu_core, 'user'), Line 701: 'cpuSys': compute_cpu_usage(cpu_core, 'sys'),
Francesco Romani has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 6:
(1 comment)
https://gerrit.ovirt.org/#/c/46269/6//COMMIT_MSG Commit Message:
Line 26: 'nodeIndex': 0}, Line 27: '1': {'cpuIdle': '96.07', Line 28: 'cpuSys': '0.47', Line 29: 'cpuUser': '3.46', Line 30: 'nodeIndex': 0}}
The indentation make it hard to understand this format - it is
+1, it seems nicer to me as well. Line 31: [...] Line 32: Line 33: When only one CPU out of two has been online before libvird was Line 34: started, vdsm reports:
Nir Soffer has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 6:
(1 comment)
https://gerrit.ovirt.org/#/c/46269/6/vdsm/virt/sampling.py File vdsm/virt/sampling.py:
Line 693: for cpu_core in cpu_cores: Line 694: # Do not try to collect cpu core data when no samples are present Line 695: if (not last_sample.cpuCores.getCoreSample(cpu_core) or not Line 696: first_sample.cpuCores.getCoreSample(cpu_core)): Line 697: continue
Yes, but I'm under the impression that we are supposed to have samples for
I don't think cpu becoming offline or online is exceptional. It is may be unusual, but it is an expected condition.
However I think we should have info for all cpus always, to instead of "continue", we should use:
_, cpu_map, _ = libvirt.getCPUMap()
for cpu_core in cpu_cores:
info = {'nodeIndex': int(node_index), 'online': cpu_map[int(cpu_core)]}
if have samples: add samples stats...
cpu_core_stats[cpu_core] = info Line 698: core_stat = { Line 699: 'nodeIndex': int(node_index), Line 700: 'cpuUser': compute_cpu_usage(cpu_core, 'user'), Line 701: 'cpuSys': compute_cpu_usage(cpu_core, 'sys'),
Roman Mohr has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 6:
(1 comment)
https://gerrit.ovirt.org/#/c/46269/6//COMMIT_MSG Commit Message:
Line 26: 'nodeIndex': 0}, Line 27: '1': {'cpuIdle': '96.07', Line 28: 'cpuSys': '0.47', Line 29: 'cpuUser': '3.46', Line 30: 'nodeIndex': 0}}
The indentation make it hard to understand this format - it is
After thinking about it, I am not for the solution where we have offline cpus in the list:
1) This is definitely not a backward compatible change for clients (Also not for the current engine).
2) The cpu was never in the list when a cpu was already offline before libvirtd came up
3) Is this really nicer for clients? Because in the current implementation they can easily calculate the overall cpu usage by iterating over the list. All fields are mandatory => very easy to use.
4) We also only count online cpu cores in the capabilities call. onlineCpus always has the same number of entries as the value of cpuCores.
Any thoughts? Line 31: [...] Line 32: Line 33: When only one CPU out of two has been online before libvird was Line 34: started, vdsm reports:
Roman Mohr has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 6:
(1 comment)
https://gerrit.ovirt.org/#/c/46269/6/vdsm/virt/sampling.py File vdsm/virt/sampling.py:
Line 693: for cpu_core in cpu_cores: Line 694: # Do not try to collect cpu core data when no samples are present Line 695: if (not last_sample.cpuCores.getCoreSample(cpu_core) or not Line 696: first_sample.cpuCores.getCoreSample(cpu_core)): Line 697: continue
Yes, but I'm under the impression that we are supposed to have samples for Besides that, why you'd like to avoid exceptions? code style, code clarity, performance, something else?
Hm, I could definitely increase the readability. Maybe I should rewrite it:
if _samples_exist(first_sample, last_sample, cpu_core): add samples
Why shouldn't I just check if my samples are there if I can? It is much safer than relying on an exception which is delegated through X levels (Yes, just two level here and the method is a helper method just for this context, but still ...) and you don't catch something you didn't want to catch. And in my eyes it is more meaningful. Line 698: core_stat = { Line 699: 'nodeIndex': int(node_index), Line 700: 'cpuUser': compute_cpu_usage(cpu_core, 'user'), Line 701: 'cpuSys': compute_cpu_usage(cpu_core, 'sys'),
Francesco Romani has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 6:
(3 comments)
https://gerrit.ovirt.org/#/c/46269/6//COMMIT_MSG Commit Message:
Line 26: 'nodeIndex': 0}, Line 27: '1': {'cpuIdle': '96.07', Line 28: 'cpuSys': '0.47', Line 29: 'cpuUser': '3.46', Line 30: 'nodeIndex': 0}}
After thinking about it, I am not for the solution where we have offline cp
1. right, but nowadays are consistent as side effect, because the getVdsStats is either correct or completely absent (due to the error). How does VDSM handle the case on which we start with one offline cpu? we have an hole in the sequence, right? (like, with cpu #2 offline: 0, 1, 3, 4, 5, 6, 7)
2. see above
3. Yes, if everything works (= all cpus online), otherwise I'm suspecting we have (and always had) a silent breakage. If backward compatibility requires to have all fields to be present, we can have placeholder data that Engine can process.
4. ok
You're right to raise backward compatibility concerns, but we should also take in account that if a cpu is put offline, it just not works (tm) :) Line 31: [...] Line 32: Line 33: When only one CPU out of two has been online before libvird was Line 34: started, vdsm reports:
https://gerrit.ovirt.org/#/c/46269/6/vdsm/virt/sampling.py File vdsm/virt/sampling.py:
Line 693: for cpu_core in cpu_cores: Line 694: # Do not try to collect cpu core data when no samples are present Line 695: if (not last_sample.cpuCores.getCoreSample(cpu_core) or not Line 696: first_sample.cpuCores.getCoreSample(cpu_core)): Line 697: continue
I don't think cpu becoming offline or online is exceptional. It is may be u
Nir:L Yep. I wrote "exceptional", I meant "unusual". Line 698: core_stat = { Line 699: 'nodeIndex': int(node_index), Line 700: 'cpuUser': compute_cpu_usage(cpu_core, 'user'), Line 701: 'cpuSys': compute_cpu_usage(cpu_core, 'sys'),
Line 693: for cpu_core in cpu_cores: Line 694: # Do not try to collect cpu core data when no samples are present Line 695: if (not last_sample.cpuCores.getCoreSample(cpu_core) or not Line 696: first_sample.cpuCores.getCoreSample(cpu_core)): Line 697: continue
Besides that, why you'd like to avoid exceptions? code style, code clarit
Roman: I kinda disagree, but I see your point and since I can't bring an objection different from personal taste, I wont' object with this approach. Line 698: core_stat = { Line 699: 'nodeIndex': int(node_index), Line 700: 'cpuUser': compute_cpu_usage(cpu_core, 'user'), Line 701: 'cpuSys': compute_cpu_usage(cpu_core, 'sys'),
automation@ovirt.org has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 7:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Roman Mohr has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 6:
(2 comments)
https://gerrit.ovirt.org/#/c/46269/6//COMMIT_MSG Commit Message:
Line 26: 'nodeIndex': 0}, Line 27: '1': {'cpuIdle': '96.07', Line 28: 'cpuSys': '0.47', Line 29: 'cpuUser': '3.46', Line 30: 'nodeIndex': 0}}
The indentation make it hard to understand this format - it is
Indention improved Line 31: [...] Line 32: Line 33: When only one CPU out of two has been online before libvird was Line 34: started, vdsm reports:
Line 29: 'cpuUser': '3.46', Line 30: 'nodeIndex': 0}} Line 31: [...] Line 32: Line 33: When only one CPU out of two has been online before libvird was
typo: libvirtd
Done Line 34: started, vdsm reports: Line 35: Line 36: $ vdsClient -s 0 getVdsStats Line 37: [...]
Francesco Romani has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 7: Code-Review+1
(1 comment)
preliminary ACK, seems OK
https://gerrit.ovirt.org/#/c/46269/7/vdsm/virt/sampling.py File vdsm/virt/sampling.py:
Line 693: for cpu_core in cpu_cores: Line 694: # Do not try to collect cpu core data when no samples are present Line 695: if (not last_sample.cpuCores.getCoreSample(cpu_core) or not Line 696: first_sample.cpuCores.getCoreSample(cpu_core)): Line 697: continue ok, it is consistent with what we do with interfaces (below), and I don't really mind here if we go LBYL instead of EAFP ( https://docs.python.org/2/glossary.html#term-eafp https://docs.python.org/2/glossary.html#term-lbyl ) Line 698: core_stat = { Line 699: 'nodeIndex': int(node_index), Line 700: 'cpuUser': compute_cpu_usage(cpu_core, 'user'), Line 701: 'cpuSys': compute_cpu_usage(cpu_core, 'sys'),
Nir Soffer has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 7:
(1 comment)
https://gerrit.ovirt.org/#/c/46269/7//COMMIT_MSG Commit Message:
Line 57: 'cpuSys': '1.40', Line 58: 'cpuUser': '7.86', Line 59: 'nodeIndex': 0} Line 60: } Line 61: [...] And what happens when cpu was offline before libvirtd started, and it goes online after that?
Vdsm will report now a new cpu to engine - does it break old engines? Line 62: Line 63: Change-Id: Ia9c247f9138e02a9230a0849a04cb2e1705e7fac
Roman Mohr has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 7:
(1 comment)
https://gerrit.ovirt.org/#/c/46269/7//COMMIT_MSG Commit Message:
Line 57: 'cpuSys': '1.40', Line 58: 'cpuUser': '7.86', Line 59: 'nodeIndex': 0} Line 60: } Line 61: [...]
And what happens when cpu was offline before libvirtd started, and it goes And what happens when cpu was offline before libvirtd started, and it goes online after that?
It will not be reported until libvirt is restarted. But even if they would, it would not be a problem.
Vdsm will report now a new cpu to engine - does it break old engines?
Neither the changed count of online cpus , nor new cpu statistics would break the engine. Only reporting partially filled statistics would break old engines. After the statistics are parsed, they are just displayed in numa widgets in the frontend. Line 62: Line 63: Change-Id: Ia9c247f9138e02a9230a0849a04cb2e1705e7fac
automation@ovirt.org has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 8:
* Update tracker::#1264003::OK * Check Bug-Url::OK * Check Public Bug::#1264003::OK, public bug * Check Product::#1264003::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Roman Mohr has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 8:
ping
automation@ovirt.org has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 9:
* #1264003::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1264003::OK, public bug * Check Product::#1264003::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Roman Mohr has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 9: Verified+1
Verified by:
1) vdsClient -s 0 getVdsStats shows all cpu cores 2) disable cpu core 3: echo 0 > /sys/devices/system/cpu/cpu3/online 3) vdsClient -s 0 getVdsStats does no longer show cpu3 4) enable cpu core 3: echo 1 > /sys/devices/system/cpu/cpu3/online 5) repeatedly check getVdsStats until two samples are present 6) vdsClient -s 0 getVdsStats shows all cpu cores again
Francesco Romani has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 9: Code-Review-1
(1 comment)
unfortunately has to be -1. I'm actually fine with all of this patch except one implementation detail.
I'm fully aware that EAFP vs LBYL is mostly matter of taste (and that the first is actually more pythonic), nevertheless the stats subsystem is heavily LBYL, and I'd like to keep it that way, OR to change all of it at once.
For the near/mid term we chosse the LBYL way, hence the score and the requirement.
https://gerrit.ovirt.org/#/c/46269/9/vdsm/virt/hoststats.py File vdsm/virt/hoststats.py:
Line 103: last_sample.cpuCores.getCoreSample(cpu_core)[mode] - Line 104: first_sample.cpuCores.getCoreSample(cpu_core)[mode] Line 105: ) % JIFFIES_BOUND Line 106: return ("%.2f" % (jiffies / interval)) Line 107: except TypeError: this is the only thing that bothers me in this patch. We spent some time removing except TypError and except AttributeError, and I'd like to avoid to reintroduce them so early, and without a very good reason to do so.
I believe that here we can make the code a little less dense and increase a bit in clarity, adding a few ifs to guard for None. Line 108: return None Line 109: Line 110: cpu_core_stats = {} Line 111: for node_index, numa_node in six.iteritems(caps.getNumaTopology()):
Nir Soffer has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 9: Code-Review-1
(6 comments)
https://gerrit.ovirt.org/#/c/46269/9/tests/hoststatsTests.py File tests/hoststatsTests.py:
Line 107: Line 108: Line 109: class HostStatsThreadTests(TestCaseBase): Line 110: Line 111: def _expectedResult(self, node_index=0): Having default value only make the tests harder to understand.
Better require all caller to specify the node_index, will make the tests more readable.
Also _expectedResult is meaningless, isn't this cpuStats dict? Line 112: return { Line 113: str(node_index): { Line 114: 'cpuIdle': '100.00', Line 115: 'cpuSys': '0.00',
Line 125: }, Line 126: 1: { Line 127: 'cpus': [1] Line 128: } Line 129: } Why is this so verbose? What is wrong with:
{0: {'cpus': [0]}, 1: {'cpus': [1]}}
And why do we need a function to return this in tests? Line 130: Line 131: def testCpuCoreStats(self): Line 132: cpu_sample = {'user': 1.0, 'sys': 2.0} Line 133:
Line 137: with MonkeyPatchScope([(caps, 'getNumaTopology', Line 138: self._fakeNumaTopology)]): Line 139: result = hoststats._get_cpu_core_stats(first_sample, last_sample) Line 140: self.assertEqual(result['0'], self._expectedResult()['0']) Line 141: self.assertEqual(result['1'], self._expectedResult(1)['1']) You are using only one dict in each assert, so I don't understand why _expectedResults is needed at all.
This is much more clear:
expected = {'cpuIdle': 100.0, ...} self.assertEqual(result['1'], expected) Line 142: Line 143: def testSkipStatsOnMissingFirstSample(self): Line 144: cpu_sample = {'user': 1.0, 'sys': 2.0} Line 145:
Line 149: with MonkeyPatchScope([(caps, 'getNumaTopology', Line 150: self._fakeNumaTopology)]): Line 151: self.assertEqual( Line 152: hoststats._get_cpu_core_stats(first_sample, last_sample), Line 153: self._expectedResult()) Again we are using one dict, better inline it. Line 154: Line 155: def testSkipStatsOnMissingLastSample(self): Line 156: cpu_sample = {'user': 1.0, 'sys': 2.0} Line 157:
Line 161: with MonkeyPatchScope([(caps, 'getNumaTopology', Line 162: self._fakeNumaTopology)]): Line 163: self.assertEqual( Line 164: hoststats._get_cpu_core_stats(first_sample, last_sample), Line 165: self._expectedResult()) Same Line 166: Line 167: def testOutputWithNoSamples(self): Line 168: expected = { Line 169: 'cpuIdle': 100.0,
https://gerrit.ovirt.org/#/c/46269/9/vdsm/virt/hoststats.py File vdsm/virt/hoststats.py:
Line 103: last_sample.cpuCores.getCoreSample(cpu_core)[mode] - Line 104: first_sample.cpuCores.getCoreSample(cpu_core)[mode] Line 105: ) % JIFFIES_BOUND Line 106: return ("%.2f" % (jiffies / interval)) Line 107: except TypeError:
this is the only thing that bothers me in this patch. We spent some time re
Please never check None with TypeError - write code that reveal your intent. Line 108: return None Line 109: Line 110: cpu_core_stats = {} Line 111: for node_index, numa_node in six.iteritems(caps.getNumaTopology()):
automation@ovirt.org has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 10:
* #1264003::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1264003::OK, public bug * Check Product::#1264003::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
automation@ovirt.org has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 11:
* #1264003::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1264003::OK, public bug * Check Product::#1264003::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Roman Mohr has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 9:
(6 comments)
https://gerrit.ovirt.org/#/c/46269/9/tests/hoststatsTests.py File tests/hoststatsTests.py:
Line 107: Line 108: Line 109: class HostStatsThreadTests(TestCaseBase): Line 110: Line 111: def _expectedResult(self, node_index=0):
Having default value only make the tests harder to understand.
Done Line 112: return { Line 113: str(node_index): { Line 114: 'cpuIdle': '100.00', Line 115: 'cpuSys': '0.00',
Line 125: }, Line 126: 1: { Line 127: 'cpus': [1] Line 128: } Line 129: }
Why is this so verbose? What is wrong with:
Should I use lambda, or am I using the MonkeyPatchScope wrong?
Made less verbose Line 130: Line 131: def testCpuCoreStats(self): Line 132: cpu_sample = {'user': 1.0, 'sys': 2.0} Line 133:
Line 137: with MonkeyPatchScope([(caps, 'getNumaTopology', Line 138: self._fakeNumaTopology)]): Line 139: result = hoststats._get_cpu_core_stats(first_sample, last_sample) Line 140: self.assertEqual(result['0'], self._expectedResult()['0']) Line 141: self.assertEqual(result['1'], self._expectedResult(1)['1'])
You are using only one dict in each assert, so I don't understand why _expe
Rearanged it a little bit. Line 142: Line 143: def testSkipStatsOnMissingFirstSample(self): Line 144: cpu_sample = {'user': 1.0, 'sys': 2.0} Line 145:
Line 149: with MonkeyPatchScope([(caps, 'getNumaTopology', Line 150: self._fakeNumaTopology)]): Line 151: self.assertEqual( Line 152: hoststats._get_cpu_core_stats(first_sample, last_sample), Line 153: self._expectedResult())
Again we are using one dict, better inline it.
Done Line 154: Line 155: def testSkipStatsOnMissingLastSample(self): Line 156: cpu_sample = {'user': 1.0, 'sys': 2.0} Line 157:
Line 161: with MonkeyPatchScope([(caps, 'getNumaTopology', Line 162: self._fakeNumaTopology)]): Line 163: self.assertEqual( Line 164: hoststats._get_cpu_core_stats(first_sample, last_sample), Line 165: self._expectedResult())
Same
Done Line 166: Line 167: def testOutputWithNoSamples(self): Line 168: expected = { Line 169: 'cpuIdle': 100.0,
https://gerrit.ovirt.org/#/c/46269/9/vdsm/virt/hoststats.py File vdsm/virt/hoststats.py:
Line 103: last_sample.cpuCores.getCoreSample(cpu_core)[mode] - Line 104: first_sample.cpuCores.getCoreSample(cpu_core)[mode] Line 105: ) % JIFFIES_BOUND Line 106: return ("%.2f" % (jiffies / interval)) Line 107: except TypeError:
Please never check None with TypeError - write code that reveal your intent
Done Line 108: return None Line 109: Line 110: cpu_core_stats = {} Line 111: for node_index, numa_node in six.iteritems(caps.getNumaTopology()):
Roman Mohr has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 9:
(1 comment)
https://gerrit.ovirt.org/#/c/46269/9/tests/hoststatsTests.py File tests/hoststatsTests.py:
Line 149: with MonkeyPatchScope([(caps, 'getNumaTopology', Line 150: self._fakeNumaTopology)]): Line 151: self.assertEqual( Line 152: hoststats._get_cpu_core_stats(first_sample, last_sample), Line 153: self._expectedResult())
Done
Not inlined, but using dict directly Line 154: Line 155: def testSkipStatsOnMissingLastSample(self): Line 156: cpu_sample = {'user': 1.0, 'sys': 2.0} Line 157:
automation@ovirt.org has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 12:
* #1264003::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1264003::OK, public bug * Check Product::#1264003::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Francesco Romani has posted comments on this change.
Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 12: Code-Review+1
thanks for the updates! At first glance looks fine now, will have another deeper look later today.
vdsm-patches@lists.fedorahosted.org