2011/5/18 Matthias Bolte <matthias.bolte@xxxxxxxxxxxxxx>:
> 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
>
The initial patch has been superseded by this series:
https://www.redhat.com/archives/libvir-list/2011-May/msg01194.html
Matthias
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list