2011/5/18 Daniel Veillard <veillard@xxxxxxxxxx>:
> On Tue, May 17, 2011 at 09:46:34AM -0600, Eric Blake wrote:
>> On 05/17/2011 07:14 AM, Daniel Veillard wrote:
>> > On Tue, May 17, 2011 at 02:45:44PM +0200, Matthias Bolte wrote:
>> >> ---
>> >> Âsrc/libvirt.c | Â100 +++++++++++++++++++++++++++++++++++++++++++++++++++++---
>> >> Â1 files changed, 94 insertions(+), 6 deletions(-)
>> >
>> >> @@ -5040,6 +5045,12 @@ virDomainGetSchedulerParameters(virDomainPtr domain,
>> >> Â Â Â Â ÂvirDispatchError(NULL);
>> >> Â Â Â Â Âreturn -1;
>> >> Â Â Â}
>> >> +
>> >> + Â Âif (params == NULL || nparams == NULL) {
>> >> + Â Â Â ÂvirLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
>> >> + Â Â Â Âgoto error;
>> >> + Â Â}
>> >
>> > Â I was just wondering here if params being NULL couldn't be used to
>> > Â find out the correct value for nparams, but looking at least at the
>> > Â qemu driver code we really expect the array there.
>>
>> Let's fix that as a separate patch.
>>
>> >
>> >> Â Â Âconn = domain->conn;
>> >>
>> >> Â Â Âif (conn->driver->domainGetSchedulerParameters) {
>> >> @@ -5084,6 +5095,12 @@ virDomainSetSchedulerParameters(virDomainPtr domain,
>> >> Â Â Â Â ÂvirDispatchError(NULL);
>> >> Â Â Â Â Âreturn -1;
>> >> Â Â Â}
>> >> +
>> >> + Â Âif (params == NULL) {
>> >> + Â Â Â ÂvirLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
>> >> + Â Â Â Âgoto error;
>> >> + Â Â}
>>
>> Hmm, here we document that nparams can be <= the value returned by
>> virDomainGetSchedulerType; which means it can be 0, which means that
>> params can be NULL. ÂI think we should change this to allow NULL,0 in
>> input as a way of querying the proper nparams size on output.
>>
>> This affects Hu's patch series, where we are adding
>> virDomainSetSchedulerParametersFlags, which should have the same semantics.
>
> I was somehow remembering that we had let open the possibility to not
> provide the full array. Looking at the function comment which is the API
> contract there was nothing about it, so I looked at qemu backend and
> it fails if nparam != 1 , so at least that driver implemented the strict
> API.
> ÂWe need to decide either way, possibly by looking first at all API
> entry points using that mechanism array pointer + array size, and see
> if we are likely to break any app if we make it strict. Or we could
> decide to allow 0 and NULL and in that case we need to document
> explicitely the semantic.
> ÂHowever for virDomainGetSchedulerParameters() it's relatively clear
> (well that could possibly be improved a bit) that the application must
> call virDomainGetSchedulerType() first to get nparams value, then
> allocate accordingly), so I would side on imposing the strict behaviour
> but make it a bit clearer.
>
> Daniel
I'm working on a series to clean this up and go for the stricter
semantic of the Get*Parameters functions.
Matthias
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list