From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40591) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZAMeM-0003BO-Gv for qemu-devel@nongnu.org; Wed, 01 Jul 2015 14:22:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZAMeI-0007fp-Bj for qemu-devel@nongnu.org; Wed, 01 Jul 2015 14:22:22 -0400 Received: from aserp1040.oracle.com ([141.146.126.69]:24522) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZAMeI-0007fR-5f for qemu-devel@nongnu.org; Wed, 01 Jul 2015 14:22:18 -0400 Date: Wed, 1 Jul 2015 14:22:09 -0400 From: Konrad Rzeszutek Wilk Message-ID: <20150701182209.GA22498@l.oracle.com> References: <1435605913-23826-1-git-send-email-konrad.wilk@oracle.com> <1435605913-23826-5-git-send-email-konrad.wilk@oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH RFC 4/6] xen: Print and use errno where applicable. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefano Stabellini Cc: xen-devel@lists.xenproject.org, qemu-devel@nongnu.org On Wed, Jul 01, 2015 at 02:01:07PM +0100, Stefano Stabellini wrote: > On Mon, 29 Jun 2015, Konrad Rzeszutek Wilk wrote: > > In Xen 4.6 commit cd2f100f0f61b3f333d52d1737dd73f02daee592 > > "libxc: Fix do_memory_op to return negative value on errors" > > made the libxc API less odd-ball: On errors, return value is > > -1 and error code is in errno. On success the return value > > is either 0 or an positive value. > > > > Since we could be running with an old toolstack in which the > > Exx value is in rc or the newer, we print both and return > > the -EXX depending on rc == -1 condition. > > > > Signed-off-by: Konrad Rzeszutek Wilk > > --- > > xen-hvm.c | 10 ++++++---- > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > diff --git a/xen-hvm.c b/xen-hvm.c > > index 0408462..a92bc14 100644 > > --- a/xen-hvm.c > > +++ b/xen-hvm.c > > @@ -345,11 +345,12 @@ go_physmap: > > unsigned long idx = pfn + i; > > xen_pfn_t gpfn = start_gpfn + i; > > > > + /* In Xen 4.6 rc is -1 and errno contains the error value. */ > > rc = xc_domain_add_to_physmap(xen_xc, xen_domid, XENMAPSPACE_gmfn, idx, gpfn); > > if (rc) { > > DPRINTF("add_to_physmap MFN %"PRI_xen_pfn" to PFN %" > > - PRI_xen_pfn" failed: %d\n", idx, gpfn, rc); > > - return -rc; > > + PRI_xen_pfn" failed: %d (errno: %d)\n", idx, gpfn, rc, errno); > > + return rc == -1 ? -errno : -rc; > > Printing both rc and errno is the right thing to do, but I am not sure > changing return value depending on the libxc version is a good idea. > Maybe we should be consistent and always return rc? In Xen 4.5 and earlier this function would return -EINVAL (say rc=EINVAL). With Xen 4.6 it would always return 1 on errors (rc is -1, and with --1 we get 1), while the errno would have EINVAL. To be consistent and have this function return an proper -Exx value we need that check to use errno in case rc == -1. I am uncomfortable with returning positive values as errors, which reminds me - I need to update the commit to mention the return 1 issue. > > > > } > > } > > > > @@ -422,11 +423,12 @@ static int xen_remove_from_physmap(XenIOState *state, > > xen_pfn_t idx = start_addr + i; > > xen_pfn_t gpfn = phys_offset + i; > > > > + /* In Xen 4.6 rc is -1 and errno contains the error value. */ > > rc = xc_domain_add_to_physmap(xen_xc, xen_domid, XENMAPSPACE_gmfn, idx, gpfn); > > if (rc) { > > fprintf(stderr, "add_to_physmap MFN %"PRI_xen_pfn" to PFN %" > > - PRI_xen_pfn" failed: %d\n", idx, gpfn, rc); > > - return -rc; > > + PRI_xen_pfn" failed: %d (errno: %d)\n", idx, gpfn, rc, errno); > > + return rc == -1 ? -errno : -rc; > > } > > } > > > > -- > > 2.1.0 > >