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: Mon, 24 Jan 2011 18:53:14 +0000 Message-ID: <1295895194.28333.34.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> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Stefano Stabellini Cc: Christoph Egger , "xen-devel@lists.xensource.com" , Ian Jackson List-Id: xen-devel@lists.xenproject.org On Mon, 2011-01-24 at 18:39 +0000, Stefano Stabellini wrote: > On Mon, 24 Jan 2011, Gianni Tedesco wrote: > > 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. > > > > -- > > xl: Check domain existance when doing domain identifier lookups > > > > Also fix a mis-formatted error message in xl destroy command. > > > > Signed-off-by: Gianni Tedesco > > > > diff -r b59f04eb8978 tools/libxl/xl_cmdimpl.c > > --- a/tools/libxl/xl_cmdimpl.c Fri Jan 21 18:06:23 2011 +0000 > > +++ b/tools/libxl/xl_cmdimpl.c Mon Jan 24 17:58:20 2011 +0000 > > @@ -143,11 +143,24 @@ 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; > > + } > > + > > + rc = libxl_domain_info(&ctx, &dominfo, *domid_r); > > + if ( rc ) > > + return rc; > > + > > + return 0; > > is this really needed? > libxl_name_to_domid should have returned error already if the domain doesn't exist. Only if was_name is true... If they pass a number then we need to check explicitly. Perhaps it would be better to put that check in an else {} statement though. Gianni -- xl: Check domain existance when doing domain identifier lookups Also fix a mis-formatted error message in xl destroy command. Signed-off-by: Gianni Tedesco diff -r b59f04eb8978 tools/libxl/xl_cmdimpl.c --- a/tools/libxl/xl_cmdimpl.c Fri Jan 21 18:06:23 2011 +0000 +++ b/tools/libxl/xl_cmdimpl.c Mon Jan 24 18:50:32 2011 +0000 @@ -143,11 +143,24 @@ 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; + 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{ + libxl_dominfo dominfo; + rc = libxl_domain_info(&ctx, &dominfo, *domid_r); + if ( rc ) + return rc; + } + + return 0; } static int cpupool_qualifier_to_cpupoolid(const char *p, uint32_t *poolid_r, @@ -2176,7 +2189,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)