xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/11] Return failure on failure for more xl commands
@ 2016-03-30 15:02 Paulina Szubarczyk
  2016-03-30 15:02 ` [PATCH 01/11] libxl_pci: improve return codes " Paulina Szubarczyk
                   ` (11 more replies)
  0 siblings, 12 replies; 16+ messages in thread
From: Paulina Szubarczyk @ 2016-03-30 15:02 UTC (permalink / raw)
  To: roger.pau, George.Dunlap, xen-devel
  Cc: Dario Faggioli, Paulina Szubarczyk, Ian Campbell

This patch includes the changes from a patch prepared by George Dunlap
[0] and expands them to more xl commands.

This is my bite-sized outreachy project [1][2].

Return failure when the command failed for more xl commands:
- mem-set
- cd-insert
- pci-*
-- freemem
-- tmem-*

This makes xl more useful for scripting.

In the case of mem-set, it means first cleaning up
libxl_set_memory_target() to return useful error codes.

For pci-* functions libxl__create_pci_backend(), libxl__device_pci_destroy_all()
return error codes instead of always 0.

Changes:
- Remove block-attach patch
- Split out removal of spurious getinfolist to a separate patch
- Try to follow CODING_STYLE more closely:
 - In general, don't initialize rc / r, but use set-and-goto
 - Use 'r' for non-libxl error codes
 - Use EXIT_FAILURE and EXIT_SUCCESS rather than magic constants in main_foo()
 - Use 1 and 0 in internal functions of xl

[0] http://lists.xenproject.org/archives/html/xen-devel/2015-12/msg02246.html
[1] http://lists.xenproject.org/archives/html/xen-devel/2016-03/msg03031.html
[2] https://www.mail-archive.com/xen-devel@lists.xen.org/msg62055.html

CC:	Wei Liu <wei.liu2@citrix.com>
CC:	Ian Jackson <ian.jackson@eu.citrix.com>
CC: Dario Faggioli <dario.faggioli@citrix.com>
CC: Ian Campbell <ian.campbell@citrix.com>

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 01/11] libxl_pci: improve return codes for more xl commands
  2016-03-30 15:02 [PATCH 00/11] Return failure on failure for more xl commands Paulina Szubarczyk
@ 2016-03-30 15:02 ` Paulina Szubarczyk
  2016-03-30 15:02 ` [PATCH 02/11] libxl_pci: clean an unused return variable Paulina Szubarczyk
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Paulina Szubarczyk @ 2016-03-30 15:02 UTC (permalink / raw)
  To: roger.pau, George.Dunlap, xen-devel
  Cc: Dario Faggioli, Paulina Szubarczyk, Ian Campbell

Return error code instead of always 0. Remove assigned-only
ret variable in libxl__create_pci_backend.

Signed-off-by: Paulina Szubarczyk <paulinaszubarczyk@gmail.com>

CC:	Wei Liu <wei.liu2@citrix.com>
CC:	Ian Jackson <ian.jackson@eu.citrix.com>
CC: Dario Faggioli <dario.faggioli@citrix.com>
CC: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/libxl_pci.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index dc10cb7..3435ce2 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -84,13 +84,11 @@ int libxl__create_pci_backend(libxl__gc *gc, uint32_t domid,
     flexarray_t *front = NULL;
     flexarray_t *back = NULL;
     libxl__device device;
-    int ret = ERROR_NOMEM, i;
+    int i;

     front = flexarray_make(gc, 16, 1);
     back = flexarray_make(gc, 16, 1);

-    ret = 0;
-
     LOG(DEBUG, "Creating pci backend");

     /* add pci device */
@@ -108,12 +106,10 @@ int libxl__create_pci_backend(libxl__gc *gc, uint32_t domid,
     flexarray_append_pair(front, "backend-id", GCSPRINTF("%d", 0));
     flexarray_append_pair(front, "state", GCSPRINTF("%d", XenbusStateInitialising));

-    libxl__device_generic_add(gc, XBT_NULL, &device,
+    return libxl__device_generic_add(gc, XBT_NULL, &device,
                               libxl__xs_kvs_of_flexarray(gc, back, back->count),
                               libxl__xs_kvs_of_flexarray(gc, front, front->count),
                               NULL);
-
-    return 0;
 }

 static int libxl__device_pci_add_xenstore(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, int starting)
--
1.9.1

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 02/11] libxl_pci: clean an unused return variable
  2016-03-30 15:02 [PATCH 00/11] Return failure on failure for more xl commands Paulina Szubarczyk
  2016-03-30 15:02 ` [PATCH 01/11] libxl_pci: improve return codes " Paulina Szubarczyk
@ 2016-03-30 15:02 ` Paulina Szubarczyk
  2016-03-30 15:02 ` [PATCH 03/11] libxl_pci: Return error code for more pci-* functions Paulina Szubarczyk
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Paulina Szubarczyk @ 2016-03-30 15:02 UTC (permalink / raw)
  To: roger.pau, George.Dunlap, xen-devel
  Cc: Dario Faggioli, Paulina Szubarczyk, Ian Campbell

libxl_device_pci_assignable_list returns:
- list of the libxl_device_pci
- NULL in case of error or lack of devices

the rc variable is unused as return code.

Signed-off-by: Paulina Szubarczyk <paulinaszubarczyk@gmail.com>

CC:	Wei Liu <wei.liu2@citrix.com>
CC:	Ian Jackson <ian.jackson@eu.citrix.com>
CC: Dario Faggioli <dario.faggioli@citrix.com>
CC: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/libxl_pci.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 3435ce2..6051ee4 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -397,12 +397,11 @@ libxl_device_pci *libxl_device_pci_assignable_list(libxl_ctx *ctx, int *num)
     libxl_device_pci *pcidevs = NULL, *new, *assigned;
     struct dirent *de;
     DIR *dir;
-    int rc, num_assigned;
+    int num_assigned;

     *num = 0;

-    rc = get_all_assigned_devices(gc, &assigned, &num_assigned);
-    if ( rc )
+    if ( get_all_assigned_devices(gc, &assigned, &num_assigned) )
         goto out;

     dir = opendir(SYSFS_PCIBACK_DRIVER);
--
1.9.1

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 03/11] libxl_pci: Return error code for more pci-* functions
  2016-03-30 15:02 [PATCH 00/11] Return failure on failure for more xl commands Paulina Szubarczyk
  2016-03-30 15:02 ` [PATCH 01/11] libxl_pci: improve return codes " Paulina Szubarczyk
  2016-03-30 15:02 ` [PATCH 02/11] libxl_pci: clean an unused return variable Paulina Szubarczyk
@ 2016-03-30 15:02 ` Paulina Szubarczyk
  2016-03-30 15:02 ` [PATCH 04/11] xl: improve return code for freemem function Paulina Szubarczyk
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Paulina Szubarczyk @ 2016-03-30 15:02 UTC (permalink / raw)
  To: roger.pau, George.Dunlap, xen-devel
  Cc: Dario Faggioli, Paulina Szubarczyk, Ian Campbell

Return rc value instead of always 0.

Signed-off-by: Paulina Szubarczyk <paulinaszubarczyk@gmail.com>

CC:	Wei Liu <wei.liu2@citrix.com>
CC:	Ian Jackson <ian.jackson@eu.citrix.com>
CC: Dario Faggioli <dario.faggioli@citrix.com>
CC: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/libxl_pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 6051ee4..e4a2c2c 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -1604,7 +1604,7 @@ int libxl__device_pci_destroy_all(libxl__gc *gc, uint32_t domid)
     }

     free(pcidevs);
-    return 0;
+    return rc;
 }

 int libxl__grant_vga_iomem_permission(libxl__gc *gc, const uint32_t domid,
--
1.9.1

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 04/11] xl: improve return code for freemem function
  2016-03-30 15:02 [PATCH 00/11] Return failure on failure for more xl commands Paulina Szubarczyk
                   ` (2 preceding siblings ...)
  2016-03-30 15:02 ` [PATCH 03/11] libxl_pci: Return error code for more pci-* functions Paulina Szubarczyk
@ 2016-03-30 15:02 ` Paulina Szubarczyk
  2016-03-30 15:02 ` [PATCH 05/11] xl: Make set_memory_target return an error code on failure Paulina Szubarczyk
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Paulina Szubarczyk @ 2016-03-30 15:02 UTC (permalink / raw)
  To: roger.pau, George.Dunlap, xen-devel
  Cc: Dario Faggioli, Paulina Szubarczyk, Ian Campbell

- Return 0 or 1 for freemem function
- Correct the condition of checking return values of freemem.

Signed-off-by: Paulina Szubarczyk <paulinaszubarczyk@gmail.com>

CC:	Wei Liu <wei.liu2@citrix.com>
CC:	Ian Jackson <ian.jackson@eu.citrix.com>
CC: Dario Faggioli <dario.faggioli@citrix.com>
CC: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/xl_cmdimpl.c | 26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 990d3c9..173cd28 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -2573,38 +2573,34 @@ static int preserve_domain(uint32_t *r_domid, libxl_event *event,

 static int freemem(uint32_t domid, libxl_domain_build_info *b_info)
 {
-    int rc, retries = 3;
+    int retries = 3;
     uint32_t need_memkb, free_memkb;

     if (!autoballoon)
         return 0;

-    rc = libxl_domain_need_memory(ctx, b_info, &need_memkb);
-    if (rc < 0)
-        return rc;
+    if (libxl_domain_need_memory(ctx, b_info, &need_memkb) < 0)
+        return 1;

     do {
-        rc = libxl_get_free_memory(ctx, &free_memkb);
-        if (rc < 0)
-            return rc;
+        if (libxl_get_free_memory(ctx, &free_memkb) < 0)
+            return 1;

         if (free_memkb >= need_memkb)
             return 0;

-        rc = libxl_set_memory_target(ctx, 0, free_memkb - need_memkb, 1, 0);
-        if (rc < 0)
-            return rc;
+        if (libxl_set_memory_target(ctx, 0, free_memkb - need_memkb, 1, 0) < 0)
+            return 1;

         /* wait until dom0 reaches its target, as long as we are making
          * progress */
-        rc = libxl_wait_for_memory_target(ctx, 0, 10);
-        if (rc < 0)
-            return rc;
+        if (libxl_wait_for_memory_target(ctx, 0, 10) < 0)
+            return 1;

         retries--;
     } while (retries > 0);

-    return ERROR_NOMEM;
+    return 1;
 }

 static void autoconnect_console(libxl_ctx *ctx_ignored,
@@ -2870,7 +2866,7 @@ start:

     if (domid_soft_reset == INVALID_DOMID) {
         ret = freemem(domid, &d_config.b_info);
-        if (ret < 0) {
+        if (ret) {
             fprintf(stderr, "failed to free memory for the domain\n");
             ret = ERROR_FAIL;
             goto error_out;
--
1.9.1

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 05/11] xl: Make set_memory_target return an error code on failure
  2016-03-30 15:02 [PATCH 00/11] Return failure on failure for more xl commands Paulina Szubarczyk
                   ` (3 preceding siblings ...)
  2016-03-30 15:02 ` [PATCH 04/11] xl: improve return code for freemem function Paulina Szubarczyk
@ 2016-03-30 15:02 ` Paulina Szubarczyk
  2016-03-30 15:02 ` [PATCH 06/11] xl: Return an error on failed cd-insert Paulina Szubarczyk
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Paulina Szubarczyk @ 2016-03-30 15:02 UTC (permalink / raw)
  To: roger.pau, George.Dunlap, xen-devel; +Cc: Paulina Szubarczyk, George Dunlap

Change a 'long long' to "int64_t" while we're at it.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Signed-off-by: Paulina Szubarczyk <paulinaszubarczyk@gmail.com>

 - Move rc -> shell return code translation into set_memory_{max,target}
 - Use EXIT_{SUCCESS,FAILURE} for main_mem*() function
 - Use 0/1 as return values of set_memory_{max,target}
 - Use EXIT_{SUCCESS,FAILURE} as exit code for set_memory_{max,target}
 - Note incidental type change in changelog

 CC: Wei Liu <wei.liu2@citrix.com>
 CC: Ian Jackson <ian.jackson@eu.citrix.com>
 CC: Dario Faggioli <dario.faggioli@citrix.com>
 CC: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/xl_cmdimpl.c | 41 +++++++++++++++++++++++++----------------
 1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 173cd28..a7f3956 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -3271,17 +3271,20 @@ static int def_getopt(int argc, char * const argv[],
 static int set_memory_max(uint32_t domid, const char *mem)
 {
     int64_t memorykb;
-    int rc;

     memorykb = parse_mem_size_kb(mem);
     if (memorykb == -1) {
         fprintf(stderr, "invalid memory size: %s\n", mem);
-        exit(3);
+        exit(EXIT_FAILURE);
     }

-    rc = libxl_domain_setmaxmem(ctx, domid, memorykb);
+    if (libxl_domain_setmaxmem(ctx, domid, memorykb)) {
+        fprintf(stderr, "cannot set domid %d static max memory to : %s\n",
+                domid, mem);
+        return 1;
+    }

-    return rc;
+    return 0;
 }

 int main_memmax(int argc, char **argv)
@@ -3289,7 +3292,6 @@ int main_memmax(int argc, char **argv)
     uint32_t domid;
     int opt = 0;
     char *mem;
-    int rc;

     SWITCH_FOREACH_OPT(opt, "", NULL, "mem-max", 2) {
         /* No options */
@@ -3298,26 +3300,30 @@ int main_memmax(int argc, char **argv)
     domid = find_domain(argv[optind]);
     mem = argv[optind + 1];

-    rc = set_memory_max(domid, mem);
-    if (rc) {
-        fprintf(stderr, "cannot set domid %d static max memory to : %s\n", domid, mem);
-        return 1;
+    if (set_memory_max(domid, mem)) {
+        return EXIT_FAILURE;
     }

-    return 0;
+    return EXIT_SUCCESS;
 }

-static void set_memory_target(uint32_t domid, const char *mem)
+static int set_memory_target(uint32_t domid, const char *mem)
 {
-    long long int memorykb;
+    int64_t memorykb;

     memorykb = parse_mem_size_kb(mem);
     if (memorykb == -1)  {
         fprintf(stderr, "invalid memory size: %s\n", mem);
-        exit(3);
+        exit(EXIT_FAILURE);
+    }
+
+    if (libxl_set_memory_target(ctx, domid, memorykb, 0, /* enforce */ 1)) {
+        fprintf(stderr, "cannot set domid %d dynamic max memory to : %s\n",
+                domid, mem);
+        return 1;
     }

-    libxl_set_memory_target(ctx, domid, memorykb, 0, /* enforce */ 1);
+    return 0;
 }

 int main_memset(int argc, char **argv)
@@ -3333,8 +3339,11 @@ int main_memset(int argc, char **argv)
     domid = find_domain(argv[optind]);
     mem = argv[optind + 1];

-    set_memory_target(domid, mem);
-    return 0;
+    if (set_memory_target(domid, mem)) {
+        return EXIT_FAILURE;
+    }
+
+    return EXIT_SUCCESS;
 }

 static int cd_insert(uint32_t domid, const char *virtdev, char *phys)
--
1.9.1

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 06/11] xl: Return an error on failed cd-insert
  2016-03-30 15:02 [PATCH 00/11] Return failure on failure for more xl commands Paulina Szubarczyk
                   ` (4 preceding siblings ...)
  2016-03-30 15:02 ` [PATCH 05/11] xl: Make set_memory_target return an error code on failure Paulina Szubarczyk
@ 2016-03-30 15:02 ` Paulina Szubarczyk
  2016-03-30 15:02 ` [PATCH 07/11] xl: Return error codes for pci* commands Paulina Szubarczyk
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Paulina Szubarczyk @ 2016-03-30 15:02 UTC (permalink / raw)
  To: roger.pau, George.Dunlap, xen-devel
  Cc: Wei Liu, Ian Campbell, Paulina Szubarczyk, Dario Faggioli,
	Ian Jackson, George Dunlap

This makes xl more useful in scripts.

The strange thing about this is that the internal cd_insert function
*already* returned something appropriate, and cd-eject was using it,
but cd-insert wasn't.

Also:

* Rework cd_insert to return 0 or 1 as an internal function
* Use EXIT_{SUCCESS,FAILURE} for main_cd_*() functions

* Use 'r' for non-libxl return code, as specified in CODING_STYLE

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Signed-off-by: Paulina Szubarczyk <paulinaszubarczyk@gmail.com>

CC: Wei Liu <wei.liu2@citrix.com>
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Dario Faggioli <dario.faggioli@citrix.com>
CC: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/xl_cmdimpl.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index a7f3956..e93808e 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -3352,7 +3352,7 @@ static int cd_insert(uint32_t domid, const char *virtdev, char *phys)
     char *buf = NULL;
     XLU_Config *config = 0;
     struct stat b;
-    int rc = 0;
+    int r;

     xasprintf(&buf, "vdev=%s,access=r,devtype=cdrom,target=%s",
               virtdev, phys ? phys : "");
@@ -3368,18 +3368,21 @@ static int cd_insert(uint32_t domid, const char *virtdev, char *phys)
         && stat(disk.pdev_path, &b)) {
         fprintf(stderr, "Cannot stat file: %s\n",
                 disk.pdev_path);
-        rc = 1;
+        r = 1;
         goto out;
     }

-    if (libxl_cdrom_insert(ctx, domid, &disk, NULL))
-        rc=1;
+    if (libxl_cdrom_insert(ctx, domid, &disk, NULL)) {
+        r = 1;
+        goto out;
+    }

+    r = 0;
 out:
     libxl_device_disk_dispose(&disk);
     free(buf);

-    return rc;
+    return r;
 }

 int main_cd_eject(int argc, char **argv)
@@ -3395,7 +3398,11 @@ int main_cd_eject(int argc, char **argv)
     domid = find_domain(argv[optind]);
     virtdev = argv[optind + 1];

-    return cd_insert(domid, virtdev, NULL);
+    if (cd_insert(domid, virtdev, NULL)) {
+        return EXIT_FAILURE;
+    }
+
+    return EXIT_SUCCESS;
 }

 int main_cd_insert(int argc, char **argv)
@@ -3413,8 +3420,11 @@ int main_cd_insert(int argc, char **argv)
     virtdev = argv[optind + 1];
     file = argv[optind + 2];

-    cd_insert(domid, virtdev, file);
-    return 0;
+    if (cd_insert(domid, virtdev, file)) {
+        return EXIT_FAILURE;
+    }
+
+    return EXIT_SUCCESS;
 }

 int main_console(int argc, char **argv)
--
1.9.1

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 07/11] xl: Return error codes for pci* commands
  2016-03-30 15:02 [PATCH 00/11] Return failure on failure for more xl commands Paulina Szubarczyk
                   ` (5 preceding siblings ...)
  2016-03-30 15:02 ` [PATCH 06/11] xl: Return an error on failed cd-insert Paulina Szubarczyk
@ 2016-03-30 15:02 ` Paulina Szubarczyk
  2016-03-30 15:02 ` [PATCH 08/11] xl: improve main_tmem_* return codes Paulina Szubarczyk
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Paulina Szubarczyk @ 2016-03-30 15:02 UTC (permalink / raw)
  To: roger.pau, George.Dunlap, xen-devel
  Cc: Wei Liu, Ian Campbell, Paulina Szubarczyk, Dario Faggioli,
	Ian Jackson, George Dunlap

Add return codes for pci-detach, pci-attach,
pci-asssignable-add, and pci-assignable-remove.

Returning error codes makes it easier for shell scripts to tell if a
command has failed or succeeded.

NB this violates the CODING_STYLE preference for not initializing the
return-value variable at declaration; but in these cases, having a
"goto out" that jumped over nothing but an "r = 0" seemed
a bit pointless.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Signed-off-by: Paulina Szubarczyk <paulinaszubarczyk@gmail.com>

CC: Wei Liu <wei.liu2@citrix.com>
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Dario Faggioli <dario.faggioli@citrix.com>
CC: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/xl_cmdimpl.c | 78 ++++++++++++++++++++++++++++++++++--------------
 1 file changed, 56 insertions(+), 22 deletions(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index e93808e..c70fd70 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -3514,10 +3514,11 @@ int main_pcilist(int argc, char **argv)
     return 0;
 }

-static void pcidetach(uint32_t domid, const char *bdf, int force)
+static int pcidetach(uint32_t domid, const char *bdf, int force)
 {
     libxl_device_pci pcidev;
     XLU_Config *config;
+    int r = 0;

     libxl_device_pci_init(&pcidev);

@@ -3526,15 +3527,20 @@ static void pcidetach(uint32_t domid, const char *bdf, int force)

     if (xlu_pci_parse_bdf(config, &pcidev, bdf)) {
         fprintf(stderr, "pci-detach: malformed BDF specification \"%s\"\n", bdf);
-        exit(2);
+        exit(EXIT_FAILURE);
+    }
+    if (force) {
+        if (libxl_device_pci_destroy(ctx, domid, &pcidev, 0))
+            r = 1;
+    } else {
+        if (libxl_device_pci_remove(ctx, domid, &pcidev, 0))
+            r = 1;
     }
-    if (force)
-        libxl_device_pci_destroy(ctx, domid, &pcidev, 0);
-    else
-        libxl_device_pci_remove(ctx, domid, &pcidev, 0);

     libxl_device_pci_dispose(&pcidev);
     xlu_cfg_destroy(config);
+
+    return r;
 }

 int main_pcidetach(int argc, char **argv)
@@ -3553,13 +3559,18 @@ int main_pcidetach(int argc, char **argv)
     domid = find_domain(argv[optind]);
     bdf = argv[optind + 1];

-    pcidetach(domid, bdf, force);
-    return 0;
+    if (pcidetach(domid, bdf, force)) {
+        return EXIT_FAILURE;
+    }
+
+    return EXIT_SUCCESS;
 }
-static void pciattach(uint32_t domid, const char *bdf, const char *vs)
+
+static int pciattach(uint32_t domid, const char *bdf, const char *vs)
 {
     libxl_device_pci pcidev;
     XLU_Config *config;
+    int r = 0;

     libxl_device_pci_init(&pcidev);

@@ -3568,12 +3579,16 @@ static void pciattach(uint32_t domid, const char *bdf, const char *vs)

     if (xlu_pci_parse_bdf(config, &pcidev, bdf)) {
         fprintf(stderr, "pci-attach: malformed BDF specification \"%s\"\n", bdf);
-        exit(2);
+        exit(EXIT_FAILURE);
     }
-    libxl_device_pci_add(ctx, domid, &pcidev, 0);
+
+    if (libxl_device_pci_add(ctx, domid, &pcidev, 0))
+        r = 1;

     libxl_device_pci_dispose(&pcidev);
     xlu_cfg_destroy(config);
+
+    return 1;
 }

 int main_pciattach(int argc, char **argv)
@@ -3592,8 +3607,11 @@ int main_pciattach(int argc, char **argv)
     if (optind + 1 < argc)
         vs = argv[optind + 2];

-    pciattach(domid, bdf, vs);
-    return 0;
+    if (pciattach(domid, bdf, vs)) {
+        return EXIT_FAILURE;
+    }
+
+    return EXIT_SUCCESS;
 }

 static void pciassignable_list(void)
@@ -3625,10 +3643,11 @@ int main_pciassignable_list(int argc, char **argv)
     return 0;
 }

-static void pciassignable_add(const char *bdf, int rebind)
+static int pciassignable_add(const char *bdf, int rebind)
 {
     libxl_device_pci pcidev;
     XLU_Config *config;
+    int r = 0;

     libxl_device_pci_init(&pcidev);

@@ -3637,12 +3656,16 @@ static void pciassignable_add(const char *bdf, int rebind)

     if (xlu_pci_parse_bdf(config, &pcidev, bdf)) {
         fprintf(stderr, "pci-assignable-add: malformed BDF specification \"%s\"\n", bdf);
-        exit(2);
+        exit(EXIT_FAILURE);
     }
-    libxl_device_pci_assignable_add(ctx, &pcidev, rebind);
+
+    if (libxl_device_pci_assignable_add(ctx, &pcidev, rebind))
+        r = 1;

     libxl_device_pci_dispose(&pcidev);
     xlu_cfg_destroy(config);
+
+    return r;
 }

 int main_pciassignable_add(int argc, char **argv)
@@ -3656,14 +3679,18 @@ int main_pciassignable_add(int argc, char **argv)

     bdf = argv[optind];

-    pciassignable_add(bdf, 1);
-    return 0;
+    if (pciassignable_add(bdf, 1)) {
+        return EXIT_FAILURE;
+    }
+
+    return EXIT_SUCCESS;
 }

-static void pciassignable_remove(const char *bdf, int rebind)
+static int pciassignable_remove(const char *bdf, int rebind)
 {
     libxl_device_pci pcidev;
     XLU_Config *config;
+    int r = 0;

     libxl_device_pci_init(&pcidev);

@@ -3674,10 +3701,14 @@ static void pciassignable_remove(const char *bdf, int rebind)
         fprintf(stderr, "pci-assignable-remove: malformed BDF specification \"%s\"\n", bdf);
         exit(2);
     }
-    libxl_device_pci_assignable_remove(ctx, &pcidev, rebind);
+
+    if (libxl_device_pci_assignable_remove(ctx, &pcidev, rebind))
+        r = 1;

     libxl_device_pci_dispose(&pcidev);
     xlu_cfg_destroy(config);
+
+    return r;
 }

 int main_pciassignable_remove(int argc, char **argv)
@@ -3694,8 +3725,11 @@ int main_pciassignable_remove(int argc, char **argv)

     bdf = argv[optind];

-    pciassignable_remove(bdf, rebind);
-    return 0;
+    if (pciassignable_remove(bdf, rebind)) {
+        return EXIT_FAILURE;
+    }
+
+    return EXIT_SUCCESS;
 }

 static void pause_domain(uint32_t domid)
--
1.9.1

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 08/11] xl: improve main_tmem_* return codes
  2016-03-30 15:02 [PATCH 00/11] Return failure on failure for more xl commands Paulina Szubarczyk
                   ` (6 preceding siblings ...)
  2016-03-30 15:02 ` [PATCH 07/11] xl: Return error codes for pci* commands Paulina Szubarczyk
@ 2016-03-30 15:02 ` Paulina Szubarczyk
  2016-03-30 15:02 ` [PATCH 09/11] libxl: Remove pointless hypercall from libxl_set_memory_target Paulina Szubarczyk
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Paulina Szubarczyk @ 2016-03-30 15:02 UTC (permalink / raw)
  To: roger.pau, George.Dunlap, xen-devel
  Cc: Wei Liu, Dario Faggioli, Paulina Szubarczyk, Ian Jackson, Ian Campbell

Functions libxl_tmem_freeze(), libxl_tmem_thaw(), libxl_tmem_set() and
libxl_tmem_shared_auth() located in libxl.c file return
ERROR_FAIL/ERROR_INVAL or internal error codes from libxc library
* improve main_tmem_* return codes by returning EXIT_{SUCCESS/FAILURE}
  accordingly to return codes of those functions.

Signed-off-by: Paulina Szubarczyk <paulinaszubarczyk@gmail.com>

CC: Wei Liu <wei.liu2@citrix.com>
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Dario Faggioli <dario.faggioli@citrix.com>
CC: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/xl_cmdimpl.c | 51 ++++++++++++++++++++++++++++++------------------
 1 file changed, 32 insertions(+), 19 deletions(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index c70fd70..348391f 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -7205,7 +7205,7 @@ int main_tmem_freeze(int argc, char **argv)
     if (!dom && all == 0) {
         fprintf(stderr, "You must specify -a or a domain id.\n\n");
         help("tmem-freeze");
-        return 1;
+        return EXIT_FAILURE;
     }

     if (all)
@@ -7213,8 +7213,11 @@ int main_tmem_freeze(int argc, char **argv)
     else
         domid = find_domain(dom);

-    libxl_tmem_freeze(ctx, domid);
-    return 0;
+    if (libxl_tmem_freeze(ctx, domid) < 0) {
+        return EXIT_FAILURE;
+    }
+
+    return EXIT_SUCCESS;
 }

 int main_tmem_thaw(int argc, char **argv)
@@ -7234,7 +7237,7 @@ int main_tmem_thaw(int argc, char **argv)
     if (!dom && all == 0) {
         fprintf(stderr, "You must specify -a or a domain id.\n\n");
         help("tmem-thaw");
-        return 1;
+        return EXIT_FAILURE;
     }

     if (all)
@@ -7242,8 +7245,11 @@ int main_tmem_thaw(int argc, char **argv)
     else
         domid = find_domain(dom);

-    libxl_tmem_thaw(ctx, domid);
-    return 0;
+    if (libxl_tmem_thaw(ctx, domid) < 0) {
+        return EXIT_FAILURE;
+    }
+
+    return EXIT_SUCCESS;
 }

 int main_tmem_set(int argc, char **argv)
@@ -7254,6 +7260,7 @@ int main_tmem_set(int argc, char **argv)
     int opt_w = 0, opt_c = 0, opt_p = 0;
     int all = 0;
     int opt;
+    int r = 0;

     SWITCH_FOREACH_OPT(opt, "aw:c:p:", NULL, "tmem-set", 0) {
     case 'a':
@@ -7277,7 +7284,7 @@ int main_tmem_set(int argc, char **argv)
     if (!dom && all == 0) {
         fprintf(stderr, "You must specify -a or a domain id.\n\n");
         help("tmem-set");
-        return 1;
+        return EXIT_FAILURE;
     }

     if (all)
@@ -7288,17 +7295,21 @@ int main_tmem_set(int argc, char **argv)
     if (!opt_w && !opt_c && !opt_p) {
         fprintf(stderr, "No set value specified.\n\n");
         help("tmem-set");
-        return 1;
+        return EXIT_FAILURE;
     }

     if (opt_w)
-        libxl_tmem_set(ctx, domid, "weight", weight);
+        r = libxl_tmem_set(ctx, domid, "weight", weight);
     if (opt_c)
-        libxl_tmem_set(ctx, domid, "cap", cap);
+        r = libxl_tmem_set(ctx, domid, "cap", cap);
     if (opt_p)
-        libxl_tmem_set(ctx, domid, "compress", compress);
+        r = libxl_tmem_set(ctx, domid, "compress", compress);

-    return 0;
+    if (r < 0) {
+        return EXIT_FAILURE;
+    }
+
+    return EXIT_SUCCESS;
 }

 int main_tmem_shared_auth(int argc, char **argv)
@@ -7328,7 +7339,7 @@ int main_tmem_shared_auth(int argc, char **argv)
     if (!dom && all == 0) {
         fprintf(stderr, "You must specify -a or a domain id.\n\n");
         help("tmem-shared-auth");
-        return 1;
+        return EXIT_FAILURE;
     }

     if (all)
@@ -7339,18 +7350,20 @@ int main_tmem_shared_auth(int argc, char **argv)
     if (uuid == NULL || autharg == NULL) {
         fprintf(stderr, "No uuid or auth specified.\n\n");
         help("tmem-shared-auth");
-        return 1;
+        return EXIT_FAILURE;
     }

     auth = strtol(autharg, &endptr, 10);
     if (*endptr != '\0') {
         fprintf(stderr, "Invalid auth, valid auth are <0|1>.\n\n");
-        return 1;
+        return EXIT_FAILURE;
     }

-    libxl_tmem_shared_auth(ctx, domid, uuid, auth);
+    if (libxl_tmem_shared_auth(ctx, domid, uuid, auth) < 0) {
+        return EXIT_FAILURE;
+    }

-    return 0;
+    return EXIT_SUCCESS;
 }

 int main_tmem_freeable(int argc, char **argv)
@@ -7364,10 +7377,10 @@ int main_tmem_freeable(int argc, char **argv)

     mb = libxl_tmem_freeable(ctx);
     if (mb == -1)
-        return -1;
+        return EXIT_FAILURE;

     printf("%d\n", mb);
-    return 0;
+    return EXIT_SUCCESS;
 }

 int main_cpupoolcreate(int argc, char **argv)
--
1.9.1

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 09/11] libxl: Remove pointless hypercall from libxl_set_memory_target
  2016-03-30 15:02 [PATCH 00/11] Return failure on failure for more xl commands Paulina Szubarczyk
                   ` (7 preceding siblings ...)
  2016-03-30 15:02 ` [PATCH 08/11] xl: improve main_tmem_* return codes Paulina Szubarczyk
@ 2016-03-30 15:02 ` Paulina Szubarczyk
  2016-03-30 15:02 ` [PATCH 10/11] libxl: Fix libxl_set_memory_target return value Paulina Szubarczyk
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Paulina Szubarczyk @ 2016-03-30 15:02 UTC (permalink / raw)
  To: roger.pau, George.Dunlap, xen-devel
  Cc: Wei Liu, Ian Campbell, Paulina Szubarczyk, Dario Faggioli,
	Ian Jackson, George Dunlap

There's no obvious reason for the call to
xc_domain_getinfolist -- all it seems to be doing is checking that the domain
exists; but if it doesn't exist, it will have already failed by this point.

NB that this will change the return value for libxl_set_memory_target:
now it will return 0 on success, rather than returning 1 (which was
the previous behavior).  This is more in line with expected behavior,
and also allows the caller to distingiush between success and other
failure modes (some of which also return 1).

Signed-off-by: George Dunlap <george.dunlap@citrix.com>

CC: Wei Liu <wei.liu2@citrix.com>
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Dario Faggioli <dario.faggioli@citrix.com>
CC: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/libxl.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 93e228d..4291b57 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -4789,11 +4789,6 @@ retry_transaction:

     libxl__xs_printf(gc, t, GCSPRINTF("%s/memory/target", dompath),
                      "%"PRIu32, new_target_memkb);
-    rc = xc_domain_getinfolist(ctx->xch, domid, 1, &info);
-    if (rc != 1 || info.domain != domid) {
-        abort_transaction = 1;
-        goto out;
-    }

     libxl_dominfo_init(&ptr);
     xcinfo2xlinfo(ctx, &info, &ptr);
--
1.9.1

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 10/11] libxl: Fix libxl_set_memory_target return value
  2016-03-30 15:02 [PATCH 00/11] Return failure on failure for more xl commands Paulina Szubarczyk
                   ` (8 preceding siblings ...)
  2016-03-30 15:02 ` [PATCH 09/11] libxl: Remove pointless hypercall from libxl_set_memory_target Paulina Szubarczyk
@ 2016-03-30 15:02 ` Paulina Szubarczyk
  2016-03-30 15:02 ` [PATCH 11/11] libxl: libxl_tmem functions improving coding style Paulina Szubarczyk
  2016-03-30 15:14 ` [PATCH 00/11] Return failure on failure for more xl commands Wei Liu
  11 siblings, 0 replies; 16+ messages in thread
From: Paulina Szubarczyk @ 2016-03-30 15:02 UTC (permalink / raw)
  To: roger.pau, George.Dunlap, xen-devel
  Cc: Wei Liu, Ian Campbell, Paulina Szubarczyk, Dario Faggioli,
	Ian Jackson, George Dunlap

libxl_set_memory_target seems to have the following return values:

* 1 on failure, if the failure happens because of a xenstore error *or*
* invalid
target

* -1 if the setmaxmem hypercall

* -errno if the set_pod_target hypercall target fails

* 0 on success

Make it consistently return ERROR_FAIL on failure, unless the
parameters were invalid, in which case return ERROR_INVAL.

In accordance with CODING_SYTLE:

1. Leave rc uninitialized, and set when an error is detected

2. Use 'r' for return values to functions whose return values are a
different error space (like xc_domain_setmaxmem and
xc_domain_set_pod_target), or where a failure means retry, rather than
fail the whole function (libxl__fill_dom0_memory_info), to reduce the
risk that

Signed-off-by: George Dunlap <george.dunlap@citrix.com>

CC: Wei Liu <wei.liu2@citrix.com>
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Dario Faggioli <dario.faggioli@citrix.com>
CC: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/libxl.c | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 4291b57..6bdf3b1 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -4672,7 +4672,7 @@ int libxl_set_memory_target(libxl_ctx *ctx, uint32_t domid,
         int32_t target_memkb, int relative, int enforce)
 {
     GC_INIT(ctx);
-    int rc = 1, abort_transaction = 0;
+    int rc, r, abort_transaction = 0;
     uint64_t memorykb;
     uint32_t videoram = 0;
     uint32_t current_target_memkb = 0, new_target_memkb = 0;
@@ -4700,15 +4700,15 @@ retry_transaction:
     if (!target && !domid) {
         if (!xs_transaction_end(ctx->xsh, t, 1))
             goto out_no_transaction;
-        rc = libxl__fill_dom0_memory_info(gc, &current_target_memkb,
+        r = libxl__fill_dom0_memory_info(gc, &current_target_memkb,
                                           &current_max_memkb);
-        if (rc < 0)
-            goto out_no_transaction;
+        if (r < 0) { rc = ERROR_FAIL; goto out_no_transaction; }
         goto retry_transaction;
     } else if (!target) {
         LOGE(ERROR, "cannot get target memory info from %s/memory/target",
              dompath);
         abort_transaction = 1;
+        rc = ERROR_FAIL;
         goto out;
     } else {
         current_target_memkb = strtoul(target, &endptr, 10);
@@ -4716,6 +4716,7 @@ retry_transaction:
             LOGE(ERROR, "invalid memory target %s from %s/memory/target\n",
                  target, dompath);
             abort_transaction = 1;
+            rc = ERROR_FAIL;
             goto out;
         }
     }
@@ -4724,6 +4725,7 @@ retry_transaction:
         LOGE(ERROR, "cannot get memory info from %s/memory/static-max",
              dompath);
         abort_transaction = 1;
+        rc = ERROR_FAIL;
         goto out;
     }
     memorykb = strtoul(memmax, &endptr, 10);
@@ -4731,6 +4733,7 @@ retry_transaction:
         LOGE(ERROR, "invalid max memory %s from %s/memory/static-max\n",
              memmax, dompath);
         abort_transaction = 1;
+        rc = ERROR_FAIL;
         goto out;
     }

@@ -4750,6 +4753,7 @@ retry_transaction:
             "memory_dynamic_max must be less than or equal to"
             " memory_static_max\n");
         abort_transaction = 1;
+        rc = ERROR_INVAL;
         goto out;
     }

@@ -4757,33 +4761,36 @@ retry_transaction:
         LOG(ERROR, "new target %d for dom0 is below the minimum threshold",
             new_target_memkb);
         abort_transaction = 1;
+        rc = ERROR_INVAL;
         goto out;
     }

     if (enforce) {
         memorykb = new_target_memkb + videoram;
-        rc = xc_domain_setmaxmem(ctx->xch, domid, memorykb +
+        r = xc_domain_setmaxmem(ctx->xch, domid, memorykb +
                 LIBXL_MAXMEM_CONSTANT);
-        if (rc != 0) {
+        if (r != 0) {
             LOGE(ERROR,
                  "xc_domain_setmaxmem domid=%u memkb=%"PRIu64" failed ""rc=%d\n",
                  domid,
                  memorykb + LIBXL_MAXMEM_CONSTANT,
-                 rc);
+                 r);
             abort_transaction = 1;
+            rc = ERROR_FAIL;
             goto out;
         }
     }

-    rc = xc_domain_set_pod_target(ctx->xch, domid,
+    r = xc_domain_set_pod_target(ctx->xch, domid,
             (new_target_memkb + LIBXL_MAXMEM_CONSTANT) / 4, NULL, NULL, NULL);
-    if (rc != 0) {
+    if (r != 0) {
         LOGE(ERROR,
              "xc_domain_set_pod_target domid=%d, memkb=%d ""failed rc=%d\n",
              domid,
              new_target_memkb / 4,
-             rc);
+             r);
         abort_transaction = 1;
+        rc = ERROR_FAIL;
         goto out;
     }

@@ -4797,6 +4804,7 @@ retry_transaction:
                      "%"PRIu32, new_target_memkb / 1024);
     libxl_dominfo_dispose(&ptr);

+    rc = 0;
 out:
     if (!xs_transaction_end(ctx->xsh, t, abort_transaction)
         && !abort_transaction)
--
1.9.1

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 11/11] libxl: libxl_tmem functions improving coding style
  2016-03-30 15:02 [PATCH 00/11] Return failure on failure for more xl commands Paulina Szubarczyk
                   ` (9 preceding siblings ...)
  2016-03-30 15:02 ` [PATCH 10/11] libxl: Fix libxl_set_memory_target return value Paulina Szubarczyk
@ 2016-03-30 15:02 ` Paulina Szubarczyk
  2016-03-30 15:14 ` [PATCH 00/11] Return failure on failure for more xl commands Wei Liu
  11 siblings, 0 replies; 16+ messages in thread
From: Paulina Szubarczyk @ 2016-03-30 15:02 UTC (permalink / raw)
  To: roger.pau, George.Dunlap, xen-devel
  Cc: Wei Liu, Dario Faggioli, Paulina Szubarczyk, Ian Jackson, Ian Campbell

In accordance with CODING_SYTLE:
 - Use 'r' for return values to functions whose return values are a
   different error space (like xc_tmem_control, xc_tmem_auth)

Signed-off-by: Paulina Szubarczyk <paulinaszubarczyk@gmail.com>

CC: Wei Liu <wei.liu2@citrix.com>
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Dario Faggioli <dario.faggioli@citrix.com>
CC: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/libxl.c | 58 ++++++++++++++++++++++++++---------------------------
 1 file changed, 29 insertions(+), 29 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 6bdf3b1..eddd39b 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -6046,14 +6046,14 @@ uint32_t libxl_vm_get_start_time(libxl_ctx *ctx, uint32_t domid)

 char *libxl_tmem_list(libxl_ctx *ctx, uint32_t domid, int use_long)
 {
-    int rc;
+    int r;
     char _buf[32768];
     GC_INIT(ctx);

-    rc = xc_tmem_control(ctx->xch, -1, XEN_SYSCTL_TMEM_OP_LIST, domid, 32768, use_long,
+    r = xc_tmem_control(ctx->xch, -1, XEN_SYSCTL_TMEM_OP_LIST, domid, 32768, use_long,
                          _buf);
-    if (rc < 0) {
-        LOGEV(ERROR, rc, "Can not get tmem list");
+    if (r < 0) {
+        LOGEV(ERROR, r, "Can not get tmem list");
         GC_FREE;
         return NULL;
     }
@@ -6064,36 +6064,36 @@ char *libxl_tmem_list(libxl_ctx *ctx, uint32_t domid, int use_long)

 int libxl_tmem_freeze(libxl_ctx *ctx, uint32_t domid)
 {
-    int rc;
+    int r;
     GC_INIT(ctx);

-    rc = xc_tmem_control(ctx->xch, -1, XEN_SYSCTL_TMEM_OP_FREEZE, domid, 0, 0,
+    r = xc_tmem_control(ctx->xch, -1, XEN_SYSCTL_TMEM_OP_FREEZE, domid, 0, 0,
                          NULL);
-    if (rc < 0) {
-        LOGEV(ERROR, rc, "Can not freeze tmem pools");
+    if (r < 0) {
+        LOGEV(ERROR, r, "Can not freeze tmem pools");
         GC_FREE;
         return ERROR_FAIL;
     }

     GC_FREE;
-    return rc;
+    return r;
 }

 int libxl_tmem_thaw(libxl_ctx *ctx, uint32_t domid)
 {
-    int rc;
+    int r;
     GC_INIT(ctx);

-    rc = xc_tmem_control(ctx->xch, -1, XEN_SYSCTL_TMEM_OP_THAW, domid, 0, 0,
+    r = xc_tmem_control(ctx->xch, -1, XEN_SYSCTL_TMEM_OP_THAW, domid, 0, 0,
                          NULL);
-    if (rc < 0) {
-        LOGEV(ERROR, rc, "Can not thaw tmem pools");
+    if (r < 0) {
+        LOGEV(ERROR, r, "Can not thaw tmem pools");
         GC_FREE;
         return ERROR_FAIL;
     }

     GC_FREE;
-    return rc;
+    return r;
 }

 static int32_t tmem_setop_from_string(char *set_name)
@@ -6110,7 +6110,7 @@ static int32_t tmem_setop_from_string(char *set_name)

 int libxl_tmem_set(libxl_ctx *ctx, uint32_t domid, char* name, uint32_t set)
 {
-    int rc;
+    int r;
     int32_t subop = tmem_setop_from_string(name);
     GC_INIT(ctx);

@@ -6119,48 +6119,48 @@ int libxl_tmem_set(libxl_ctx *ctx, uint32_t domid, char* name, uint32_t set)
         GC_FREE;
         return ERROR_INVAL;
     }
-    rc = xc_tmem_control(ctx->xch, -1, subop, domid, set, 0, NULL);
-    if (rc < 0) {
-        LOGEV(ERROR, rc, "Can not set tmem %s", name);
+    r = xc_tmem_control(ctx->xch, -1, subop, domid, set, 0, NULL);
+    if (r < 0) {
+        LOGEV(ERROR, r, "Can not set tmem %s", name);
         GC_FREE;
         return ERROR_FAIL;
     }

     GC_FREE;
-    return rc;
+    return r;
 }

 int libxl_tmem_shared_auth(libxl_ctx *ctx, uint32_t domid,
                            char* uuid, int auth)
 {
-    int rc;
+    int r;
     GC_INIT(ctx);

-    rc = xc_tmem_auth(ctx->xch, domid, uuid, auth);
-    if (rc < 0) {
-        LOGEV(ERROR, rc, "Can not set tmem shared auth");
+    r = xc_tmem_auth(ctx->xch, domid, uuid, auth);
+    if (r < 0) {
+        LOGEV(ERROR, r, "Can not set tmem shared auth");
         GC_FREE;
         return ERROR_FAIL;
     }

     GC_FREE;
-    return rc;
+    return r;
 }

 int libxl_tmem_freeable(libxl_ctx *ctx)
 {
-    int rc;
+    int r;
     GC_INIT(ctx);

-    rc = xc_tmem_control(ctx->xch, -1, XEN_SYSCTL_TMEM_OP_QUERY_FREEABLE_MB, -1, 0, 0, 0);
-    if (rc < 0) {
-        LOGEV(ERROR, rc, "Can not get tmem freeable memory");
+    r = xc_tmem_control(ctx->xch, -1, XEN_SYSCTL_TMEM_OP_QUERY_FREEABLE_MB, -1, 0, 0, 0);
+    if (r < 0) {
+        LOGEV(ERROR, r, "Can not get tmem freeable memory");
         GC_FREE;
         return ERROR_FAIL;
     }

     GC_FREE;
-    return rc;
+    return r;
 }

 int libxl_get_freecpus(libxl_ctx *ctx, libxl_bitmap *cpumap)
--
1.9.1

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 00/11] Return failure on failure for more xl commands
  2016-03-30 15:02 [PATCH 00/11] Return failure on failure for more xl commands Paulina Szubarczyk
                   ` (10 preceding siblings ...)
  2016-03-30 15:02 ` [PATCH 11/11] libxl: libxl_tmem functions improving coding style Paulina Szubarczyk
@ 2016-03-30 15:14 ` Wei Liu
  2016-03-30 17:19   ` Paulina Szubarczyk
  11 siblings, 1 reply; 16+ messages in thread
From: Wei Liu @ 2016-03-30 15:14 UTC (permalink / raw)
  To: Paulina Szubarczyk
  Cc: Wei Liu, Ian Campbell, George.Dunlap, Dario Faggioli, xen-devel,
	roger.pau

On Wed, Mar 30, 2016 at 05:02:39PM +0200, Paulina Szubarczyk wrote:
> This patch includes the changes from a patch prepared by George Dunlap
> [0] and expands them to more xl commands.
> 

Hello,

Did you perhaps make a mistake? As far as I can tell some patches were
already applied. Don't worry, we all make mistakes from time to time.

I realise you might be looking at master branch. Please check staging
branch to see if you should drop some of the already patches.

And perhaps can you indicate which patches should we look at in case you
don't want to resend just yet.

Wei.

> This is my bite-sized outreachy project [1][2].
> 
> Return failure when the command failed for more xl commands:
> - mem-set
> - cd-insert
> - pci-*
> -- freemem
> -- tmem-*
> 
> This makes xl more useful for scripting.
> 
> In the case of mem-set, it means first cleaning up
> libxl_set_memory_target() to return useful error codes.
> 
> For pci-* functions libxl__create_pci_backend(), libxl__device_pci_destroy_all()
> return error codes instead of always 0.
> 
> Changes:
> - Remove block-attach patch
> - Split out removal of spurious getinfolist to a separate patch
> - Try to follow CODING_STYLE more closely:
>  - In general, don't initialize rc / r, but use set-and-goto
>  - Use 'r' for non-libxl error codes
>  - Use EXIT_FAILURE and EXIT_SUCCESS rather than magic constants in main_foo()
>  - Use 1 and 0 in internal functions of xl
> 
> [0] http://lists.xenproject.org/archives/html/xen-devel/2015-12/msg02246.html
> [1] http://lists.xenproject.org/archives/html/xen-devel/2016-03/msg03031.html
> [2] https://www.mail-archive.com/xen-devel@lists.xen.org/msg62055.html
> 
> CC:	Wei Liu <wei.liu2@citrix.com>
> CC:	Ian Jackson <ian.jackson@eu.citrix.com>
> CC: Dario Faggioli <dario.faggioli@citrix.com>
> CC: Ian Campbell <ian.campbell@citrix.com>
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 00/11] Return failure on failure for more xl commands
  2016-03-30 15:14 ` [PATCH 00/11] Return failure on failure for more xl commands Wei Liu
@ 2016-03-30 17:19   ` Paulina Szubarczyk
  2016-03-31 15:06     ` Wei Liu
  0 siblings, 1 reply; 16+ messages in thread
From: Paulina Szubarczyk @ 2016-03-30 17:19 UTC (permalink / raw)
  To: Wei Liu
  Cc: George.Dunlap, xen-devel, Dario Faggioli, Ian Campbell,
	Roger Pau Monné

Yes, I was looking at wrong branch, I am sorry. The patches

* {01-04,08,11} were not attached in the previous patch [0].

* {05,06} are applied in the version made by George Dunlap.
  I added the changes to return 0/1 in internal functions
  and EXIT_{SUCCESS/FAILURE} according to [2]. It certainly should be
  resend.

* 07 was attached in the previous patch but is not applied in staging.
  It relates pci-* functions. I modified it as in the point higher.

* 09 is applied.

* 10 is not applied, I did not modified it. It brings coding style improvements.

Summing up the new changes
* 01-03 are the patches related to libxl_pci, the functions return error code
  instead of always 0
* 04 modifies freemem from xl_cmdimpl to follow the pattern to return 0/1 for
  not main functions
* 08 is a change to return error codes for main_tmem* related functions
* 11 is a change with improvement of coding style, change 'rc -> r'

Paulina

On 30 March 2016 at 17:14, Wei Liu <wei.liu2@citrix.com> wrote:
> On Wed, Mar 30, 2016 at 05:02:39PM +0200, Paulina Szubarczyk wrote:
>> This patch includes the changes from a patch prepared by George Dunlap
>> [0] and expands them to more xl commands.
>>
>
> Hello,
>
> Did you perhaps make a mistake? As far as I can tell some patches were
> already applied. Don't worry, we all make mistakes from time to time.
>
> I realise you might be looking at master branch. Please check staging
> branch to see if you should drop some of the already patches.
>
> And perhaps can you indicate which patches should we look at in case you
> don't want to resend just yet.
>
> Wei.
>
>> This is my bite-sized outreachy project [1][2].
>>
>> Return failure when the command failed for more xl commands:
>> - mem-set
>> - cd-insert
>> - pci-*
>> -- freemem
>> -- tmem-*
>>
>> This makes xl more useful for scripting.
>>
>> In the case of mem-set, it means first cleaning up
>> libxl_set_memory_target() to return useful error codes.
>>
>> For pci-* functions libxl__create_pci_backend(), libxl__device_pci_destroy_all()
>> return error codes instead of always 0.
>>
>> Changes:
>> - Remove block-attach patch
>> - Split out removal of spurious getinfolist to a separate patch
>> - Try to follow CODING_STYLE more closely:
>>  - In general, don't initialize rc / r, but use set-and-goto
>>  - Use 'r' for non-libxl error codes
>>  - Use EXIT_FAILURE and EXIT_SUCCESS rather than magic constants in main_foo()
>>  - Use 1 and 0 in internal functions of xl
>>
>> [0] http://lists.xenproject.org/archives/html/xen-devel/2015-12/msg02246.html
>> [1] http://lists.xenproject.org/archives/html/xen-devel/2016-03/msg03031.html
>> [2] https://www.mail-archive.com/xen-devel@lists.xen.org/msg62055.html
>>
>> CC:   Wei Liu <wei.liu2@citrix.com>
>> CC:   Ian Jackson <ian.jackson@eu.citrix.com>
>> CC: Dario Faggioli <dario.faggioli@citrix.com>
>> CC: Ian Campbell <ian.campbell@citrix.com>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 00/11] Return failure on failure for more xl commands
  2016-03-30 17:19   ` Paulina Szubarczyk
@ 2016-03-31 15:06     ` Wei Liu
  2016-04-01  9:55       ` Paulina Szubarczyk
  0 siblings, 1 reply; 16+ messages in thread
From: Wei Liu @ 2016-03-31 15:06 UTC (permalink / raw)
  To: Paulina Szubarczyk
  Cc: Wei Liu, Ian Campbell, George.Dunlap, Dario Faggioli, xen-devel,
	Roger Pau Monné

On Wed, Mar 30, 2016 at 07:19:41PM +0200, Paulina Szubarczyk wrote:
> Yes, I was looking at wrong branch, I am sorry. The patches
> 
> * {01-04,08,11} were not attached in the previous patch [0].
> 
> * {05,06} are applied in the version made by George Dunlap.
>   I added the changes to return 0/1 in internal functions
>   and EXIT_{SUCCESS/FAILURE} according to [2]. It certainly should be
>   resend.
> 
> * 07 was attached in the previous patch but is not applied in staging.
>   It relates pci-* functions. I modified it as in the point higher.
> 
> * 09 is applied.
> 
> * 10 is not applied, I did not modified it. It brings coding style improvements.
> 

Given that the applied patches interleave with not-yet-applied patches I
think it's best if you can rebase on top of staging and resend.

We shall look at it in due time. Note that we're currently quite busy so
it might take us some time to get to your patches.  Do you have a
deadline that you need to meet? Deadline for OPW etc?

Wei.

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 00/11] Return failure on failure for more xl commands
  2016-03-31 15:06     ` Wei Liu
@ 2016-04-01  9:55       ` Paulina Szubarczyk
  0 siblings, 0 replies; 16+ messages in thread
From: Paulina Szubarczyk @ 2016-04-01  9:55 UTC (permalink / raw)
  To: Wei Liu
  Cc: George Dunlap, xen-devel, Dario Faggioli, Ian Campbell,
	Roger Pau Monné

I will resend them as soon as I managed to check it all once again.

At 22 April the decision of selected candidates is going to be posted.
During that time I should complete the bite-size task and according
to the https://wiki.gnome.org/Outreachy work on more contributions
if that is possible.

Paulina

On 31 March 2016 at 17:06, Wei Liu <wei.liu2@citrix.com> wrote:
> On Wed, Mar 30, 2016 at 07:19:41PM +0200, Paulina Szubarczyk wrote:
>> Yes, I was looking at wrong branch, I am sorry. The patches
>>
>> * {01-04,08,11} were not attached in the previous patch [0].
>>
>> * {05,06} are applied in the version made by George Dunlap.
>>   I added the changes to return 0/1 in internal functions
>>   and EXIT_{SUCCESS/FAILURE} according to [2]. It certainly should be
>>   resend.
>>
>> * 07 was attached in the previous patch but is not applied in staging.
>>   It relates pci-* functions. I modified it as in the point higher.
>>
>> * 09 is applied.
>>
>> * 10 is not applied, I did not modified it. It brings coding style improvements.
>>
>
> Given that the applied patches interleave with not-yet-applied patches I
> think it's best if you can rebase on top of staging and resend.
>
> We shall look at it in due time. Note that we're currently quite busy so
> it might take us some time to get to your patches.  Do you have a
> deadline that you need to meet? Deadline for OPW etc?
>
> Wei.

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2016-04-01  9:55 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-30 15:02 [PATCH 00/11] Return failure on failure for more xl commands Paulina Szubarczyk
2016-03-30 15:02 ` [PATCH 01/11] libxl_pci: improve return codes " Paulina Szubarczyk
2016-03-30 15:02 ` [PATCH 02/11] libxl_pci: clean an unused return variable Paulina Szubarczyk
2016-03-30 15:02 ` [PATCH 03/11] libxl_pci: Return error code for more pci-* functions Paulina Szubarczyk
2016-03-30 15:02 ` [PATCH 04/11] xl: improve return code for freemem function Paulina Szubarczyk
2016-03-30 15:02 ` [PATCH 05/11] xl: Make set_memory_target return an error code on failure Paulina Szubarczyk
2016-03-30 15:02 ` [PATCH 06/11] xl: Return an error on failed cd-insert Paulina Szubarczyk
2016-03-30 15:02 ` [PATCH 07/11] xl: Return error codes for pci* commands Paulina Szubarczyk
2016-03-30 15:02 ` [PATCH 08/11] xl: improve main_tmem_* return codes Paulina Szubarczyk
2016-03-30 15:02 ` [PATCH 09/11] libxl: Remove pointless hypercall from libxl_set_memory_target Paulina Szubarczyk
2016-03-30 15:02 ` [PATCH 10/11] libxl: Fix libxl_set_memory_target return value Paulina Szubarczyk
2016-03-30 15:02 ` [PATCH 11/11] libxl: libxl_tmem functions improving coding style Paulina Szubarczyk
2016-03-30 15:14 ` [PATCH 00/11] Return failure on failure for more xl commands Wei Liu
2016-03-30 17:19   ` Paulina Szubarczyk
2016-03-31 15:06     ` Wei Liu
2016-04-01  9:55       ` Paulina Szubarczyk

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).