From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [PATCH v3 1/6] xl: Return proper error codes for block-attach and block-detach Date: Tue, 8 Dec 2015 12:24:35 +0000 Message-ID: References: <1448970835-2706-1-git-send-email-george.dunlap@eu.citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1448970835-2706-1-git-send-email-george.dunlap@eu.citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: "xen-devel@lists.xen.org" Cc: George Dunlap , Ian Jackson , Wei Liu , Ian Campbell List-Id: xen-devel@lists.xenproject.org On Tue, Dec 1, 2015 at 11:53 AM, George Dunlap wrote: > 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 Ping? -George > --- > CC: Ian Campbell > CC: Ian Jackson > CC: Wei Liu > CC: Konrad Wilk > > v3: > - Rebased to tip > > v2: > - Restructure functions to make sure init and dispose are properly > called. > --- > tools/libxl/libxl.c | 2 -- > tools/libxl/xl_cmdimpl.c | 29 ++++++++++++++++++++--------- > 2 files changed, 20 insertions(+), 11 deletions(-) > > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c > index bd3aac8..814d056 100644 > --- a/tools/libxl/libxl.c > +++ b/tools/libxl/libxl.c > @@ -2738,8 +2738,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 2b6371d..2ba2393 100644 > --- a/tools/libxl/xl_cmdimpl.c > +++ b/tools/libxl/xl_cmdimpl.c > @@ -570,8 +570,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); } > @@ -3340,6 +3338,8 @@ static int cd_insert(uint32_t domid, const char *virtdev, char *phys) > xasprintf(&buf, "vdev=%s,access=r,devtype=cdrom,target=%s", > virtdev, phys ? phys : ""); > > + libxl_device_disk_init(&disk); > + > parse_disk_config(&config, buf, &disk); > > /* ATM the existence of the backing file is not checked for qdisk > @@ -6649,6 +6649,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 */ > @@ -6660,6 +6661,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); > > @@ -6668,14 +6671,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"); > - return 1; > + rc = 1; > } > - return 0; > +out: > + libxl_device_disk_dispose(&disk); > + > + return rc; > } > > int main_blocklist(int argc, char **argv) > @@ -6726,18 +6732,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"); > - return 1; > + rc = 1; > } > + > +out: > libxl_device_disk_dispose(&disk); > + > return rc; > } > > -- > 2.1.4 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel