Re: [Qemu-devel] [PATCH RFC 4/6] xen: Print and use errno where applicable.

[Stefano Stabellini]

On Wed, 1 Jul 2015, Konrad Rzeszutek Wilk wrote:
 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 konrad.w...@oracle.com
   ---
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.

Maybe the best thing to do is to introduce a versioned
xen_xc_domain_add_to_physmap to include/hw/xen/xen_common.h


 I am uncomfortable with returning positive values as errors, which reminds me 
 -
 I need to update the commit to mention the return 1 issue.

Agreed


  
  
}
}

   @@ -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
   
 

Re: [Qemu-devel] [PATCH RFC 4/6] xen: Print and use errno where applicable.

[Konrad Rzeszutek Wilk]

On Thu, Jul 02, 2015 at 12:00:29PM +0100, Stefano Stabellini wrote:
 On Wed, 1 Jul 2015, Konrad Rzeszutek Wilk wrote:
  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 konrad.w...@oracle.com
---
 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.
 
 Maybe the best thing to do is to introduce a versioned
 xen_xc_domain_add_to_physmap to include/hw/xen/xen_common.h

Aah, hadn't seen that before. Yes will do that.
 
 
  I am uncomfortable with returning positive values as errors, which reminds 
  me -
  I need to update the commit to mention the return 1 issue.
 
 Agreed
 
 
   
   
 }
 }
 
@@ -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

  

Re: [Qemu-devel] [PATCH RFC 4/6] xen: Print and use errno where applicable.

[Konrad Rzeszutek Wilk]

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 konrad.w...@oracle.com
  ---
   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
  

Re: [Qemu-devel] [PATCH RFC 4/6] xen: Print and use errno where applicable.

[Stefano Stabellini]

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 konrad.w...@oracle.com
 ---
  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?


  }
  }
  
 @@ -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
 

[Qemu-devel] [PATCH RFC 4/6] xen: Print and use errno where applicable.

[Konrad Rzeszutek Wilk]

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 konrad.w...@oracle.com
---
 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;
 }
 }
 
@@ -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