From patchwork Fri Apr 8 09:58:02 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Dario Faggioli X-Patchwork-Id: 8782571 Return-Path: X-Original-To: patchwork-xen-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id CE83DC0554 for ; Fri, 8 Apr 2016 10:00:28 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 794352026D for ; Fri, 8 Apr 2016 10:00:27 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id BB169202AE for ; Fri, 8 Apr 2016 10:00:22 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1aoTB7-0002ql-Uo; Fri, 08 Apr 2016 09:58:13 +0000 Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1aoTB6-0002qY-RA for xen-devel@lists.xenproject.org; Fri, 08 Apr 2016 09:58:13 +0000 Received: from [85.158.139.211] by server-1.bemta-5.messagelabs.com id AB/FD-29419-4B087075; Fri, 08 Apr 2016 09:58:12 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprCKsWRWlGSWpSXmKPExsXitHRDpO6mBvZ wg84ZVhbft0xmcmD0OPzhCksAYxRrZl5SfkUCa0bDvW2MBb/aGStmLBdvYHzWytjFyMkhIRAi sWr+JTYQm1fASOLi7X4WEFtYIEpi2vG5TCA2m4CBxJsde1m7GDk4RARUJG7vNehi5OJgFljCK PHr4A6wehag+Mcju5hBbE4Be4mP6yaxg9QLCRRLXPyUAxLmF5CUuPXlI1gJs0ClxNnuGSwQJ+ hLHJ+3AuoEQYmTM5+AxYUE1CRmzL3MClHDLXH79FTmCYz8s5C0z0LSAhF3kDh/5SU7hK0p0br 9N5StKDGl+yGU7SUxY983xllA1zELBEmcarHEVGIrsW7de6iRNhKbri5ghLDlJba/ncO8gJF7 FaNGcWpRWWqRrqGFXlJRZnpGSW5iZo6uoYGpXm5qcXFiempOYlKxXnJ+7iZGYAwxAMEOxqbtn ocYJTmYlER5DxawhwvxJeWnVGYkFmfEF5XmpBYfYpTh4FCS4F1cD5QTLEpNT61Iy8wBRjNMWo KDR0mEdwFImre4IDG3ODMdInWKUZdjy4Iba5mEWPLy81KlxHk3ghQJgBRllObBjYAllkuMslL CvIxARwnxFKQW5WaWoMq/YhTnYFQS5t0KMoUnM68EbtMroCOYgI64wM8GckRJIkJKqoHxiPfC d3tPhunazLl1/uGp6J5Xkb+7syLDucwfqvSeUj+/RPbiF8PLYh06uey1XApHX4c/rX7kvT3/D sebxLrNuwtFap8/i4+sDv9/z04n23GLvwBXECOviYDen50Z2zcePfhx7d3MzjVTHdbP0VLZ8t fxqFdPieHN0Gm1i53mLvzpXSeW/u+cEktxRqKhFnNRcSIAQqS4DScDAAA= X-Env-Sender: prvs=899e9c19a=dario.faggioli@citrix.com X-Msg-Ref: server-15.tower-206.messagelabs.com!1460109489!25283790!1 X-Originating-IP: [66.165.176.89] X-SpamReason: No, hits=0.0 required=7.0 tests=sa_preprocessor: VHJ1c3RlZCBJUDogNjYuMTY1LjE3Ni44OSA9PiAyMDMwMDc=\n, received_headers: No Received headers X-StarScan-Received: X-StarScan-Version: 8.28; banners=-,-,- X-VirusChecked: Checked Received: (qmail 33511 invoked from network); 8 Apr 2016 09:58:10 -0000 Received: from smtp.citrix.com (HELO SMTP.CITRIX.COM) (66.165.176.89) by server-15.tower-206.messagelabs.com with RC4-SHA encrypted SMTP; 8 Apr 2016 09:58:10 -0000 X-IronPort-AV: E=Sophos;i="5.24,449,1454976000"; d="asc'?scan'208";a="345744008" Message-ID: <1460109482.13871.83.camel@citrix.com> From: Dario Faggioli To: Date: Fri, 8 Apr 2016 11:58:02 +0200 In-Reply-To: <20160408022324.9058.21614.stgit@Solace.fritz.box> References: <20160408022241.9058.86808.stgit@Solace.fritz.box> <20160408022324.9058.21614.stgit@Solace.fritz.box> Organization: Citrix Inc. X-Mailer: Evolution 3.18.5.2 (3.18.5.2-1.fc23) MIME-Version: 1.0 X-DLP: MIA2 Cc: Wei Liu , Ian Jackson , Harmandeep Kaur , George Dunlap Subject: Re: [Xen-devel] [PATCH v3 1/6] xl: improve return and exit codes of memory related functions X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Fri, 2016-04-08 at 04:23 +0200, Dario Faggioli wrote: >  > --- a/tools/libxl/xl_cmdimpl.c > +++ b/tools/libxl/xl_cmdimpl.c > @@ -3391,7 +3396,7 @@ static int set_memory_max(uint32_t domid, const > char *mem) >      memorykb = parse_mem_size_kb(mem); >      if (memorykb == -1) { >          fprintf(stderr, "invalid memory size: %s\n", mem); > -        exit(3); > +        exit(EXIT_FAILURE); > Actually, after George's 0614c4542, it's better if this is turned into a return (and likewise in set_memory_target()). The inlined (and attached) patch does that. And there's an updated branch (labelled v4, and of course I can resubmit the whole series from there, if necessary), with this version of the patch in it, at:  git://xenbits.xen.org/people/dariof/xen.git rel/xl/exit-code-cleanup-v4  http://xenbits.xen.org/gitweb/?p=people/dariof/xen.git;a=shortlog;h=refs/heads/rel/xl/exit-code-cleanup-v4 Regards, Dario Acked-by: Wei Liu --- commit 43586222084638ec98b693702132f3412a63ddda Author: Harmandeep Kaur Date:   Thu Apr 7 12:38:09 2016 +0200     xl: improve return and exit codes of memory related functions          by making them more consistent with other examples in xl.          While there, make freemem() of boolean return type, which     looks more natural, and add comment explaining why     parse_mem_size_kb() needs to diverge from the pattern.          Signed-off-by: Harmandeep Kaur     Signed-off-by: Dario Faggioli     ---     Cc: Ian Jackson     Cc: Wei Liu     Cc: George Dunlap     ---     v4: make set_memory_max() and set_memory_target() more         consistent.          v3: Shorten changelog.         Make freemem() boolean return type.          v2: Add comment to explain return vaule of parse_mem_size_kb().         Add freemem() and main_sharing().         Remove find_domain(). commit 43586222084638ec98b693702132f3412a63ddda Author: Harmandeep Kaur Date: Thu Apr 7 12:38:09 2016 +0200 xl: improve return and exit codes of memory related functions by making them more consistent with other examples in xl. While there, make freemem() of boolean return type, which looks more natural, and add comment explaining why parse_mem_size_kb() needs to diverge from the pattern. Signed-off-by: Harmandeep Kaur Signed-off-by: Dario Faggioli --- Cc: Ian Jackson Cc: Wei Liu Cc: George Dunlap --- v4: make set_memory_max() and set_memory_target() more consistent. v3: Shorten changelog. Make freemem() boolean return type. v2: Add comment to explain return vaule of parse_mem_size_kb(). Add freemem() and main_sharing(). Remove find_domain(). diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c index 2ee6c74..eab2abd 100644 --- a/tools/libxl/xl_cmdimpl.c +++ b/tools/libxl/xl_cmdimpl.c @@ -2680,40 +2680,45 @@ static int preserve_domain(uint32_t *r_domid, libxl_event *event, return rc == 0 ? 1 : 0; } -static int freemem(uint32_t domid, libxl_domain_build_info *b_info) +/* + * Returns false if memory can't be freed, but also if we encounter errors. + * Returns true in case there is already, or we manage to free it, enough + * memory, but also if autoballoon is false. + */ +static bool freemem(uint32_t domid, libxl_domain_build_info *b_info) { int rc, retries = 3; uint32_t need_memkb, free_memkb; if (!autoballoon) - return 0; + return true; rc = libxl_domain_need_memory(ctx, b_info, &need_memkb); if (rc < 0) - return rc; + return false; do { rc = libxl_get_free_memory(ctx, &free_memkb); if (rc < 0) - return rc; + return false; if (free_memkb >= need_memkb) - return 0; + return true; rc = libxl_set_memory_target(ctx, 0, free_memkb - need_memkb, 1, 0); if (rc < 0) - return rc; + return false; /* 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; + return false; retries--; } while (retries > 0); - return ERROR_NOMEM; + return false; } static void autoconnect_console(libxl_ctx *ctx_ignored, @@ -2980,8 +2985,7 @@ start: goto error_out; if (domid_soft_reset == INVALID_DOMID) { - ret = freemem(domid, &d_config.b_info); - if (ret < 0) { + if (!freemem(domid, &d_config.b_info)) { fprintf(stderr, "failed to free memory for the domain\n"); ret = ERROR_FAIL; goto error_out; @@ -3245,6 +3249,7 @@ void help(const char *command) } } +/* Returns -1 on failure; the amount of memory on success. */ static int64_t parse_mem_size_kb(const char *mem) { char *endptr; @@ -3391,7 +3396,7 @@ static int set_memory_max(uint32_t domid, const char *mem) memorykb = parse_mem_size_kb(mem); if (memorykb == -1) { fprintf(stderr, "invalid memory size: %s\n", mem); - exit(3); + return EXIT_FAILURE; } if (libxl_domain_setmaxmem(ctx, domid, memorykb)) { @@ -3425,7 +3430,7 @@ static int set_memory_target(uint32_t domid, const char *mem) memorykb = parse_mem_size_kb(mem); if (memorykb == -1) { fprintf(stderr, "invalid memory size: %s\n", mem); - exit(3); + return EXIT_FAILURE; } if (libxl_set_memory_target(ctx, domid, memorykb, 0, /* enforce */ 1)) { @@ -6132,7 +6137,7 @@ int main_sharing(int argc, char **argv) info = libxl_list_domain(ctx, &nb_domain); if (!info) { fprintf(stderr, "libxl_list_domain failed.\n"); - return 1; + return EXIT_FAILURE; } info_free = info; } else if (optind == argc-1) { @@ -6141,17 +6146,17 @@ int main_sharing(int argc, char **argv) if (rc == ERROR_DOMAIN_NOTFOUND) { fprintf(stderr, "Error: Domain \'%s\' does not exist.\n", argv[optind]); - return -rc; + return EXIT_FAILURE; } if (rc) { fprintf(stderr, "libxl_domain_info failed (code %d).\n", rc); - return -rc; + return EXIT_FAILURE; } info = &info_buf; nb_domain = 1; } else { help("sharing"); - return 2; + return EXIT_FAILURE; } sharing(info, nb_domain); @@ -6161,7 +6166,7 @@ int main_sharing(int argc, char **argv) else libxl_dominfo_dispose(info); - return 0; + return EXIT_SUCCESS; } static int sched_domain_get(libxl_scheduler sched, int domid,