diff mbox

[v2,07/10] xl_cmdimpl - Add return codes for pci-detach, pci-attach, pci-asssignable-add, and pci-assignable-remove.

Message ID 1459943163-18697-8-git-send-email-paulinaszubarczyk@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Paulina Szubarczyk April 6, 2016, 11:46 a.m. UTC 
From: George Dunlap <George.Dunlap@eu.citrix.com>

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@eu.citrix.com>
Signed-off-by: Paulina Szubarczyk <paulinaszubarczyk@gmail.com>

---
Changed since v1:
* Changed exit() calls to 'return 1;'

---
 tools/libxl/xl_cmdimpl.c | 80 ++++++++++++++++++++++++++++++++++--------------
 1 file changed, 57 insertions(+), 23 deletions(-)

Comments

Dario Faggioli April 8, 2016, 8:36 a.m. UTC | #1 
On Wed, 2016-04-06 at 13:46 +0200, Paulina Szubarczyk wrote:
> From: George Dunlap <George.Dunlap@eu.citrix.com>
> 
> 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.
> 
AFAIUI, that rule applies to 'int rc' variables, meant at hosting libxl
error codes and not equally strictly (although it might be a nice to
have) to 'int r' and alike variables meant at hosting return code from
interna functions, syscalls, libxc calls, etc.

Unless I'm missing or misreading something from libxl's CODING_STYLE.
:-)

> Signed-off-by: George Dunlap <George.Dunlap@eu.citrix.com>
> Signed-off-by: Paulina Szubarczyk <paulinaszubarczyk@gmail.com>
> 
> ---
> Changed since v1:
> * Changed exit() calls to 'return 1;'
> 
Why do we need to do that? changing exit(<random_number>) in either
exit(EXIT_SUCCESS) or (in most of the cases) exit(EXIT_FAILURE) is much
better, and is the patter we're (slowly :-/) trying to force into xl.

Regards,
Dario
diff mbox

Patch

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index fd23f23..0bc7ad9 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -3817,10 +3817,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);
 
@@ -3829,15 +3830,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);
+        return 1;
+    }
+    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)
@@ -3856,13 +3862,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);
 
@@ -3871,12 +3882,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);
+        return 1;
     }
-    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)
@@ -3895,8 +3910,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)
@@ -3928,10 +3946,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);
 
@@ -3940,12 +3959,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);
+        return 1;
     }
-    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)
@@ -3959,14 +3982,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);
 
@@ -3975,12 +4002,16 @@  static void pciassignable_remove(const char *bdf, int rebind)
 
     if (xlu_pci_parse_bdf(config, &pcidev, bdf)) {
         fprintf(stderr, "pci-assignable-remove: malformed BDF specification \"%s\"\n", bdf);
-        exit(2);
+        return 1;
     }
-    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)
@@ -3997,8 +4028,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)