Re: [PATCH v5 for Xen 4.7 3/4] libxl: enable per-VCPU parameter settings for RTDS scheduler

[Wei Liu <wei.liu2@citrix.com>]

On Fri, Feb 05, 2016 at 06:10:33PM -0600, Chong Li wrote:
> I'll fix these coding style issues.
> 

Thank you. :-)

[...]
> >> +        num_vcpus = max_vcpuid + 1;
> >> +        GCNEW_ARRAY(vcpus, num_vcpus);
> >> +        if (sched_rtds_validate_params(gc, scinfo->vcpus[0].period,
> >> +                                 scinfo->vcpus[0].budget,
> >
> > This doesn't make sense. You take this path because scinfo->num_vcpus is
> > 0 but now you're dereferencing scinfo->vcpus[0]. Do I miss anything?
> For commands like " xl sched-rtds -d vm1 -v all -p 1000 -b 1000"
> (which sets all vcpus with
> the same scheduling parameters), we pass the budget and period via
> scinfo->vcpus[0].
> 
> I'll add more explanation here.

No, adding more explanation won't help.

Let me explain a bit. Libxl is the library that can be used by multiple
applications. Xl is just one of the applications. The other application
that I know of is libvirt.

So, the incarnation of a particular xl command is of no concern how we
define the semantics of a libxl API. That is, you can come up with an
unambiguous API but still support the same xl command.

Currently the semantics of this (new?) libxl API seems to be broken,
because you're (ab)using num_vcpus to represent a special case. In
effect you can't really whether the array is empty. When num_vcpus is 0,
you shouldn't dereference vcpus array, at all, because the semantics of
num_vcpus == 0 is that the array is empty.

Wei.

> >
> >> +                                 &vcpus[0].s.rtds.period,
> >> +                                 &vcpus[0].s.rtds.budget))
> >
> 
> -- 
> Chong Li
> Department of Computer Science and Engineering
> Washington University in St.louis