From: Dario Faggioli <dario.faggioli@citrix.com>
To: Harmandeep Kaur <write.harmandeep@gmail.com>,
xen-devel@lists.xenproject.org
Cc: ian.jackson@eu.citrix.com, stefano.stebellini@eu.citrix.com,
wei.liu2@citrix.com, ian.campbell@citrix.com,
george.dunlap@citrix.com
Subject: Re: [PATCH 3/3] xl: convert cpupool related return codes to EXIT_[SUCCESS|FAILURE]
Date: Fri, 23 Oct 2015 14:10:57 +0200 [thread overview]
Message-ID: <1445602257.5117.115.camel@citrix.com> (raw)
In-Reply-To: <1445586491-15093-3-git-send-email-write.harmandeep@gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 3925 bytes --]
And here we are at the last patch of this series.
Allow me to say that this is quite good a first contribution! Thanks
for this, and I'm looking forward to seeing version 2!! :-D
About this patch, a few comments below.
On Fri, 2015-10-23 at 13:18 +0530, Harmandeep Kaur wrote:
> turning cpupools related functions xl exit codes towards using the
> EXIT_[SUCCESS|FAILURE] macros, instead of instead of arbitrary
> numbers
> or libxl return codes.
>
> Signed-off-by: Harmandeep Kaur <write.harmandeep@gmail.com>
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -7497,7 +7497,7 @@ out:
> free(name);
> free(config_data);
> free(extra_config);
> - return rc;
> + return rc ? EXIT_FAILURE : EXIT_SUCCESS;
> }
>
I think you can just initialize rc with EXIT_FAILURE, assign
EXIT_SUCCESS to it near the end, if everything went ok, and then keep
the 'return rc';
> int main_cpupooldestroy(int argc, char **argv)
> @@ -7580,13 +7580,13 @@ int main_cpupooldestroy(int argc, char
> **argv)
> if (libxl_cpupool_qualifier_to_cpupoolid(ctx, pool, &poolid,
> NULL) ||
> !libxl_cpupoolid_is_valid(ctx, poolid)) {
> fprintf(stderr, "unknown cpupool '%s'\n", pool);
> - return 1;
> + return EXIT_FAILURE;
> }
>
> if (libxl_cpupool_destroy(ctx, poolid))
> - return 1;
> + return EXIT_FAILURE;
>
> - return 0;
> + return EXIT_SUCCESS;
> }
>
For this one: I've sent a patch for another reason yesterday, and while
there I did the exit code adjustment myself. So, update your tree and,
if my patch has been committed already, just skip this function.
https://www.mail-archive.com/xen-devel@lists.xen.org/msg42850.html
Which brings up a question: what git tree are you using for
development? You should stay either on master or staging branches (and
I recommend staging) of the official repository:
http://wiki.xenproject.org/wiki/Xen_Project_Repositories
> @@ -7653,7 +7653,7 @@ int main_cpupoolcpuadd(int argc, char **argv)
>
> out:
> libxl_bitmap_dispose(&cpumap);
> - return rc;
> + return rc ? EXIT_FAILURE : EXIT_SUCCESS;
>
Same as already said for main_cpupoolcreate, just us rc.
> @@ -7691,7 +7691,7 @@ int main_cpupoolcpuremove(int argc, char
> **argv)
>
> out:
> libxl_bitmap_dispose(&cpumap);
> - return rc;
> + return rc ? EXIT_FAILURE : EXIT_SUCCESS;
>
And here.
> int main_cpupoolnumasplit(int argc, char **argv)
> @@ -7758,7 +7758,7 @@ int main_cpupoolnumasplit(int argc, char
> **argv)
> poolinfo = libxl_list_cpupool(ctx, &n_pools);
> if (!poolinfo) {
> fprintf(stderr, "error getting cpupool info\n");
> - return 1;
> + return EXIT_FAILURE;
> }
> poolid = poolinfo[0].poolid;
> sched = poolinfo[0].sched;
> @@ -7766,13 +7766,13 @@ int main_cpupoolnumasplit(int argc, char
> **argv)
>
> if (n_pools > 1) {
> fprintf(stderr, "splitting not possible, already cpupools in
> use\n");
> - return 1;
> + return EXIT_FAILURE;
> }
>
> topology = libxl_get_cpu_topology(ctx, &n_cpus);
> if (topology == NULL) {
> fprintf(stderr, "libxl_get_topologyinfo failed\n");
> - return 1;
> + return EXIT_FAILURE;
> }
>
> if (libxl_cpu_bitmap_alloc(ctx, &cpumap, 0)) {
> @@ -7869,7 +7869,7 @@ out:
> libxl_dominfo_dispose(&info);
> free(name);
>
> - return rc;
> + return rc ? EXIT_FAILURE : EXIT_SUCCESS;
> }
>
And here too.
Thanks and regards,
Dario
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
next prev parent reply other threads:[~2015-10-23 12:11 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-23 7:48 [PATCH 1/3] xl: convert scheduling related return codes to EXIT_[SUCCESS|FAILURE] Harmandeep Kaur
2015-10-23 7:48 ` [PATCH 2/3] xl: convert vcpu " Harmandeep Kaur
2015-10-23 10:41 ` Dario Faggioli
2015-10-23 7:48 ` [PATCH 3/3] xl: convert cpupool " Harmandeep Kaur
2015-10-23 12:10 ` Dario Faggioli [this message]
2015-10-23 12:33 ` Harmandeep Kaur
2015-10-23 8:30 ` [PATCH 1/3] xl: convert scheduling " Dario Faggioli
2015-10-23 9:56 ` Ian Campbell
2015-10-23 10:03 ` Dario Faggioli
2015-10-23 10:16 ` Ian Campbell
2015-10-23 10:16 ` Dario Faggioli
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1445602257.5117.115.camel@citrix.com \
--to=dario.faggioli@citrix.com \
--cc=george.dunlap@citrix.com \
--cc=ian.campbell@citrix.com \
--cc=ian.jackson@eu.citrix.com \
--cc=stefano.stebellini@eu.citrix.com \
--cc=wei.liu2@citrix.com \
--cc=write.harmandeep@gmail.com \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.