Re: [libvirt] [PATCH 29/34] conf: Don't store vcpusched orthogonally to other vcpu info

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)) {

> + 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]

> + 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.