[Xen-devel] [PATCH v2 for 4.5] xl: Return proper error codes for block-attach and block-detach

[George Dunlap <george.dunlap@xxxxxxxxxxxxx>]

Return proper error codes on failure so that scripts can tell whether
the command completed properly or not.

Also while we're at it, normalize init and dispose of
libxl_device_disk structures.  This means calling init and dispose in
the actual functions where they are declared.

This in turn means changing parse_disk_config_multistring() to not
call libxl_device_disk_init.  And that in turn means changes to
callers of parse_disk_config().

It also means removing libxl_device_disk_init() from
libxl.c:libxl_vdev_to_device_disk().  This is only called from
xl_cmdimpl.c.

Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
---
CC: Ian Campbell <ian.campbell@xxxxxxxxxx>
CC: Ian Jackson <ian.jackson@xxxxxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>
CC: Konrad Wilk <konrad.wilk@xxxxxxxxxx>

Release justification: This is a bug fix.  It's a fairly minor one,
but it's also a very simple one.

v2:
 - Restructure functions to make sure init and dispose are properly
 called.
---
 tools/libxl/libxl.c      |  2 --
 tools/libxl/xl_cmdimpl.c | 28 +++++++++++++++++++++-------
 2 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index f7961f6..d9c4ce7 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2611,8 +2611,6 @@ int libxl_vdev_to_device_disk(libxl_ctx *ctx, uint32_t 
domid,
     if (devid < 0)
         return ERROR_INVAL;
 
-    libxl_device_disk_init(disk);
-
     dompath = libxl__xs_get_dompath(gc, domid);
     if (!dompath) {
         goto out;
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 3c9f146..25af715 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -485,8 +485,6 @@ static void parse_disk_config_multistring(XLU_Config 
**config,
 {
     int e;
 
-    libxl_device_disk_init(disk);
-
     if (!*config) {
         *config = xlu_cfg_init(stderr, "command line");
         if (!*config) { perror("xlu_cfg_init"); exit(-1); }
@@ -1405,6 +1403,7 @@ static void parse_config_data(const char *config_source,
 
             d_config->disks = (libxl_device_disk *) realloc(d_config->disks, 
sizeof (libxl_device_disk) * (d_config->num_disks + 1));
             disk = d_config->disks + d_config->num_disks;
+            libxl_device_disk_init(disk);
             parse_disk_config(&config, buf2, disk);
 
             free(buf2);
@@ -2931,6 +2930,8 @@ static int cd_insert(uint32_t domid, const char *virtdev, 
char *phys)
         return 1;
     }
 
+    libxl_device_disk_init(&disk);
+
     parse_disk_config(&config, buf, &disk);
 
     /* ATM the existence of the backing file is not checked for qdisk
@@ -6359,6 +6360,7 @@ int main_blockattach(int argc, char **argv)
     uint32_t fe_domid;
     libxl_device_disk disk;
     XLU_Config *config = 0;
+    int rc = 0;
 
     SWITCH_FOREACH_OPT(opt, "", NULL, "block-attach", 2) {
         /* No options */
@@ -6370,6 +6372,8 @@ int main_blockattach(int argc, char **argv)
     }
     optind++;
 
+    libxl_device_disk_init(&disk);
+
     parse_disk_config_multistring
         (&config, argc-optind, (const char* const*)argv + optind, &disk);
 
@@ -6378,13 +6382,17 @@ int main_blockattach(int argc, char **argv)
         printf("disk: %s\n", json);
         free(json);
         if (ferror(stdout) || fflush(stdout)) { perror("stdout"); exit(-1); }
-        return 0;
+        goto out;
     }
 
     if (libxl_device_disk_add(ctx, fe_domid, &disk, 0)) {
         fprintf(stderr, "libxl_device_disk_add failed.\n");
+        rc = 1;
     }
-    return 0;
+out:
+    libxl_device_disk_dispose(&disk);
+
+    return rc;
 }
 
 int main_blocklist(int argc, char **argv)
@@ -6435,17 +6443,23 @@ int main_blockdetach(int argc, char **argv)
         /* No options */
     }
 
+    libxl_device_disk_init(&disk);
+
     domid = find_domain(argv[optind]);
 
     if (libxl_vdev_to_device_disk(ctx, domid, argv[optind+1], &disk)) {
         fprintf(stderr, "Error: Device %s not connected.\n", argv[optind+1]);
-        return 1;
+        rc = 1;
+        goto out;
     }
-    rc = libxl_device_disk_remove(ctx, domid, &disk, 0);
-    if (rc) {
+    if (libxl_device_disk_remove(ctx, domid, &disk, 0)) {
         fprintf(stderr, "libxl_device_disk_remove failed.\n");
+        rc = 1;
     }
+
+out:
     libxl_device_disk_dispose(&disk);
+
     return rc;
 }
 
-- 
1.9.1


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