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
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list