Re: [RFC Patch v4 7/9] correct xc_domain_save()'s return value

[Wen Congyang <wency@cn.fujitsu.com>]

On 09/23/2014 05:25 PM, Andrew Cooper wrote:
> On 23/09/14 10:10, Wen Congyang wrote:
>> On 09/23/2014 05:00 PM, Andrew Cooper wrote:
>>> On 23/09/14 03:14, Wen Congyang wrote:
>>>> On 09/22/2014 10:06 PM, Andrew Cooper wrote:
>>>>> On 22/09/14 14:13, Ian Campbell wrote:
>>>>>> On Mon, 2014-09-22 at 14:03 +0100, Ian Jackson wrote:
>>>>>>> Ian Campbell writes ("Re: [Xen-devel] [RFC Patch v4 7/9] correct xc_domain_save()'s return value"):
>>>>>>>> libxc doesn't know that, if it is important then it seems like the
>>>>>>>> failure + errno ought to be marshalled across the IPC link.
>>>>>>> Yes, but ...
>>>>>>>
>>>>>>>> It may be that this can be easily handled in
>>>>>>>> libxl__srm_callout_sendreply + helper_getreply. Ian J -- what do you
>>>>>>>> think?
>>>>>>> ... while that would be possbile, we have another option.
>>>>>>>
>>>>>>> We could say that the callbacks return errno values.  That would
>>>>>>> simplify the API and avoid having the IPC involve accesses to global
>>>>>>> variables (ie, things not in the functions' parameter lists).
>>>>>>>
>>>>>>> If we do that then it becomes the responsibility of xc_domain_save to
>>>>>>> either change its own API to return errno, or to save the callback's
>>>>>>> return value in errno.
>>>>>> Hrm. libxc is already a complete mess wrt error returning/handling
>>>>>> because some proportion of the code incorrectly does/assumes this sort
>>>>>> of thing is happening (because people were confused about the syscall
>>>>>> returns from the kernel vs. process context). Having a place in libxc
>>>>>> where this is now done on purpose seems a bit like setting the rope on
>>>>>> fire to me...
>>>>>>
>>>>>> Ian.
>>>>>>
>>>>> libxc is an absolute mess, but this is far from the only codepath (even
>>>>> in xc_domain_save()) which ends up like this.
>>>>>
>>>>> The *only* safe assumption is that ==0 is success and !=0 is failure for
>>>>> xc_domain_save(), and errno may or may not be relevant, whether rc is -1
>>>>> or not.
>>>> Do you mean: errno is undefined even if rc is -1?
>>>>
>>>> Thanks
>>>> Wen Congyang
>>> Correct, last time I checked.  The error handling in libxc is in dire
>>> need of fixing from scratch.
>> For v2 version migration, we can do it like this. But xc_domain_save()
>> is a public API, and is used some years. So I am not sure if I can
>> change its behavior...
>>
>> Thanks
>> Wen Congyang
> 
> xc_domain_safe() is free to be changed.  libxc is an unstable API, and
> has been changed for every Xen release I recall.

So, if the user wants to a stable API, he should use libxl. Is it right?

Thanks
Wen Congyang

> 
> The migrationv2 code is just as guilty of using -1 and an undefined
> errno, although it does promise that there will be a relevant error in
> the log.  It stems from the same problems around the callbacks where
> there is insufficient information passed, but also from the likes of
> {read,write}_exact() which uses -1/0 for EOF.  I considered changing it
> to -1/EBADF and still not decided which is the lessor of two evils.  (As
> far as I am aware, the legacy code has the same problem.)
> 
> ~Andrew
> 
> .
>