From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [RFC Patch v4 7/9] correct xc_domain_save()'s return value Date: Tue, 23 Sep 2014 10:25:05 +0100 Message-ID: <54213C71.6090909@citrix.com> References: <1411365561-29242-1-git-send-email-wency@cn.fujitsu.com> <1411365561-29242-8-git-send-email-wency@cn.fujitsu.com> <20140922073053.GA6584@aepfle.de> <541FD0F4.8080400@cn.fujitsu.com> <20140922074625.GA9788@aepfle.de> <541FD88B.5080209@cn.fujitsu.com> <1411389776.18331.73.camel@kazak.uk.xensource.com> <21536.7722.687712.755871@mariner.uk.xensource.com> <1411391615.18331.86.camel@kazak.uk.xensource.com> <54202CED.3040508@citrix.com> <5420D792.5060406@cn.fujitsu.com> <542136A1.6030000@citrix.com> <542138FC.8070400@cn.fujitsu.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <542138FC.8070400@cn.fujitsu.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Wen Congyang , Ian Campbell , Ian Jackson Cc: Olaf Hering , Lai Jiangshan , Jiang Yunhong , Dong Eddie , xen devel , Yang Hongyang List-Id: xen-devel@lists.xenproject.org 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