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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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.

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


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel