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

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

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

> 
> For the suspend callback itself, I have encountered 3 different error
> schemes which stem from a complete lack of expectations set out beside
> the function pointer definition in xenguest.h, which is why I had to
> copy the old "!=0" check in migration v2.  Even intree in the past, -1
> and 1 have been used for errors from this function pointer.
> 
> 
> It is my opinion that it is not worth changing any of the error handling
> until someone does all of libxc and makes it all consistent.  The risk
> for introducing subtle bugs into in (and out-of-) tree callers is just
> too high, and this change alone does not fix xc_domain_save() to be
> consistent.
> 
> ~Andrew
> 
> .
>