From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gianni Tedesco Subject: Re: [PATCH, v2]: xl: Check domain existance when doing domain identifier lookups Date: Tue, 25 Jan 2011 17:35:20 +0000 Message-ID: <1295976920.25215.5.camel@qabil.uk.xensource.com> References: <1295888759.28333.14.camel@qabil.uk.xensource.com> <201101241827.46797.Christoph.Egger@amd.com> <1295891031.28333.25.camel@qabil.uk.xensource.com> <1295893129.28333.31.camel@qabil.uk.xensource.com> <19775.2104.742709.639002@mariner.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <19775.2104.742709.639002@mariner.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Ian Jackson Cc: Christoph Egger , "xen-devel@lists.xensource.com" , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org On Tue, 2011-01-25 at 17:28 +0000, Ian Jackson wrote: > Gianni Tedesco writes ("[PATCH, v2]: xl: Check domain existance when doing domain identifier lookups"): > > It occurs to me that the last patch won't fix it for anything but > > destroy. We should bail with a nice error for any command looking up a > > domain that doesn't exist and be consistent with name vs. numeric ID. > > I think the destroy logic needs to be different because if > libxl_domain_info fails for some other reason than ERROR_INVAL (ie > "domain does not exist"), destruction it needs to try all of the other > destruction steps. > > So if you do a general change like this the destroy case probably > needs to bypass it. Yes I think you are right, except for that we can make the same argument against more than just destroy. In other words, let's change the logic such that only non-existant domain causes this to fail and other errors are ignored. -- xl: Check domain existance when doing domain identifier lookups Also fix a mis-formatted error messages in xl destroy command. Signed-off-by: Gianni Tedesco diff -r 5e91e7a6b130 tools/libxl/xl_cmdimpl.c --- a/tools/libxl/xl_cmdimpl.c Tue Jan 25 14:07:39 2011 +0000 +++ b/tools/libxl/xl_cmdimpl.c Tue Jan 25 17:32:19 2011 +0000 @@ -143,11 +143,25 @@ static int qualifier_to_id(const char *p static int domain_qualifier_to_domid(const char *p, uint32_t *domid_r, int *was_name_r) { - int was_name; + libxl_dominfo dominfo; + int was_name, rc; was_name = qualifier_to_id(p, domid_r); - if (was_name_r) *was_name_r = was_name; - return was_name ? libxl_name_to_domid(&ctx, p, domid_r) : 0; + if (was_name_r) + *was_name_r = was_name; + + if ( was_name ) { + rc = libxl_name_to_domid(&ctx, p, domid_r); + if ( rc ) + return rc; + }else{ + rc = libxl_domain_info(&ctx, &dominfo, *domid_r); + /* error only if domain does not exist */ + if ( rc == ERROR_INVAL ) + return rc; + } + + return 0; } static int cpupool_qualifier_to_cpupoolid(const char *p, uint32_t *poolid_r, @@ -2176,7 +2190,7 @@ static void destroy_domain(const char *p exit(-1); } rc = libxl_domain_destroy(&ctx, domid, 0); - if (rc) { fprintf(stderr,"destroy failed (rc=%d)\n.",rc); exit(-1); } + if (rc) { fprintf(stderr,"destroy failed (rc=%d).\n",rc); exit(-1); } } static void shutdown_domain(const char *p, int wait) @@ -2185,7 +2199,7 @@ static void shutdown_domain(const char * find_domain(p); rc=libxl_domain_shutdown(&ctx, domid, 0); - if (rc) { fprintf(stderr,"shutdown failed (rc=%d)\n.",rc);exit(-1); } + if (rc) { fprintf(stderr,"shutdown failed (rc=%d)\n",rc);exit(-1); } if (wait) { libxl_waiter waiter; @@ -2227,7 +2241,7 @@ static void reboot_domain(const char *p) int rc; find_domain(p); rc=libxl_domain_shutdown(&ctx, domid, 1); - if (rc) { fprintf(stderr,"reboot failed (rc=%d)\n.",rc);exit(-1); } + if (rc) { fprintf(stderr,"reboot failed (rc=%d)\n",rc);exit(-1); } } static void list_domains_details(const libxl_dominfo *info, int nb_domain) @@ -2669,7 +2683,7 @@ static void core_dump_domain(const char int rc; find_domain(domain_spec); rc=libxl_domain_core_dump(&ctx, domid, filename); - if (rc) { fprintf(stderr,"core dump failed (rc=%d)\n.",rc);exit(-1); } + if (rc) { fprintf(stderr,"core dump failed (rc=%d)\n",rc);exit(-1); } } static void migrate_receive(int debug, int daemonize)