All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 for 4.5] xl: Return proper error codes for block-attach and block-detach
@ 2014-11-13 19:04 George Dunlap
  2014-11-14 11:12 ` Ian Campbell
  0 siblings, 1 reply; 8+ messages in thread
From: George Dunlap @ 2014-11-13 19:04 UTC (permalink / raw)
  To: xen-devel; +Cc: George Dunlap, Ian Jackson, Wei Liu, Ian Campbell

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@eu.citrix.com>
---
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Ian Jackson <ian.jackson@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Konrad Wilk <konrad.wilk@oracle.com>

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-11-20 19:46 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-13 19:04 [PATCH v2 for 4.5] xl: Return proper error codes for block-attach and block-detach George Dunlap
2014-11-14 11:12 ` Ian Campbell
2014-11-17 12:36   ` George Dunlap
2014-11-20 15:47     ` Ian Campbell
2014-11-20 15:51       ` George Dunlap
2014-11-20 15:56         ` Ian Campbell
2014-11-20 15:59           ` George Dunlap
2014-11-20 19:46       ` Konrad Rzeszutek Wilk

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.