On Sat, Jan 16, 2016 at 10:23:13 -0500, John Ferlan wrote:
On 01/14/2016 11:27 AM, Peter Krempa wrote:
Due to bad design the vcpu sched element is orthogonal to the way how the data belongs to the corresponding objects. Now that vcpus are a struct that allow to store other info too, let's convert the data to the sane structure.
The helpers for the conversion are made universal so that they can be reused for iothreads too.
This patch also resolves https://bugzilla.redhat.com/show_bug.cgi?id=1235180 since with the correct storage approach you can't have dangling data. --- src/conf/domain_conf.c | 231 +++++++++++++++++++++++++++++++++++++++--------- src/conf/domain_conf.h | 8 +- src/qemu/qemu_driver.c | 6 +- src/qemu/qemu_process.c | 8 +- 4 files changed, 202 insertions(+), 51 deletions(-)
[...]
+ for (i = VIR_PROC_POLICY_NONE + 1; i < VIR_PROC_POLICY_LAST; i++) { + virBitmapClearAll(schedMap); + + /* find vcpus using a particular scheduler */ + next = -1; + while ((next = virBitmapNextSetBit(resourceMap, next)) > -1) { + sched = func(def, next); + + if (sched->policy == i) + ignore_value(virBitmapSetBit(schedMap, next)); + } + + /* it's necessary to discriminate priority levels for schedulers that + * have them */ + while (!virBitmapIsAllClear(schedMap)) {
So start of this loop guarantees, that theres at least one element in the bitmap ...
+ virBitmapPtr currentMap = NULL; + ssize_t nextprio; + bool hasPriority = false; + int priority; + + switch ((virProcessSchedPolicy) i) { + case VIR_PROC_POLICY_NONE: + case VIR_PROC_POLICY_BATCH: + case VIR_PROC_POLICY_IDLE: + case VIR_PROC_POLICY_LAST: + currentMap = schedMap; + break; + + case VIR_PROC_POLICY_FIFO: + case VIR_PROC_POLICY_RR: + virBitmapClearAll(prioMap); + hasPriority = true; + + /* we need to find a subset of vCPUs with the given scheduler + * that share the priority */ + nextprio = virBitmapNextSetBit(schedMap, -1);
Coverity notes that virBitmapNextSetBit can return -1; however, [1]
... thus this won't return -1 in any case here. Coverity is obviously wrong as usual since it's terrible at introspecting the bitmap code.
+ sched = func(def, nextprio); + priority = sched->priority; + + ignore_value(virBitmapSetBit(prioMap, nextprio));
[1] passing a -1 'nextprio' to virBitmapSetBit as 'size_t b' cannot happen.
So this doesn't make sense. Peter