From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wen Congyang Subject: Re: [RFC Patch v4 7/9] correct xc_domain_save()'s return value Date: Tue, 23 Sep 2014 17:29:45 +0800 Message-ID: <54213D89.3080205@cn.fujitsu.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> <54213C71.6090909@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <54213C71.6090909@citrix.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: Andrew Cooper , 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 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 > > . >