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
signature.asc
Description: Digital signature
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
