Re: [PATCH v5 7/8] libxl/libxc: Move libxl_get_numainfo()'s hypercall buffer management to libxc

[Boris Ostrovsky <boris.ostrovsky@oracle.com>]

On 03/20/2015 09:56 AM, Ian Campbell wrote:
> On Thu, 2015-03-19 at 17:54 -0400, Boris Ostrovsky wrote:
>> diff --git a/tools/libxc/xc_misc.c b/tools/libxc/xc_misc.c
>> index 411128e..607ae61 100644
>> --- a/tools/libxc/xc_misc.c
>> +++ b/tools/libxc/xc_misc.c
>> @@ -209,22 +209,49 @@ out:
>>       return ret;
>>   }
>>   
>> -int xc_numainfo(xc_interface *xch,
>> -                xc_numainfo_t *put_info)
>> +int xc_numainfo(xc_interface *xch, unsigned *max_nodes,
>> +                xc_meminfo_t *meminfo, uint32_t *distance)
>>   {
>>       int ret;
>>       DECLARE_SYSCTL;
>> +    DECLARE_HYPERCALL_BOUNCE(meminfo, *max_nodes * sizeof(*meminfo),
>> +                             XC_HYPERCALL_BUFFER_BOUNCE_OUT);
>> +    DECLARE_HYPERCALL_BOUNCE(distance,
>> +                             *max_nodes * *max_nodes * sizeof(*distance),
>> +                             XC_HYPERCALL_BUFFER_BOUNCE_OUT);
>>   
>> -    sysctl.cmd = XEN_SYSCTL_numainfo;
>> +    if (meminfo && distance) {
>> +        if ((ret = xc_hypercall_bounce_pre(xch, meminfo)))
>> +            goto out;
>> +        if ((ret = xc_hypercall_bounce_pre(xch, distance)))
>> +            goto out;
> Same comment about handling NULL as before.
>
> In addition what if only one of meminfo and distance is NULL? Is that
> valid or do you need a !!meminfo ^ !!distance check?

I want to treat this as as an error here, which is why I have
         } else if (meminfo || distance) {
             errno = EINVAL;
             return -1;
         }

because the hypervisor will only attempt to copy numainfo things when 
both are valid. Otherwise (i.e. even if only one is a NULL) it will 
assume that this is a request for size. The alternative would be to add 
another error there, which I decided not to do.

-boris

>
> Rests looks ok.
>