From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andre Przywara Subject: Re: [PATCH]: xl: catch invalid numeric domain ID Date: Mon, 24 Jan 2011 23:12:21 +0100 Message-ID: <4D3DF945.4070307@amd.com> References: <4D39B142.90806@amd.com> <19769.55433.742234.824540@mariner.uk.xensource.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------030106090401090509020601" Return-path: In-Reply-To: <19769.55433.742234.824540@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: Ian Campbell , "xen-devel@lists.xensource.com" List-Id: xen-devel@lists.xenproject.org --------------030106090401090509020601 Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit On 01/21/2011 08:03 PM, Ian Jackson wrote: > Andre Przywara writes ("[PATCH]: xl: catch invalid numeric domain ID"): >> if you run any domain specific xl command with an invalid (aka >> non-existing) numeric domain ID, the command does not abort, but >> proceeds with some bogus number: > > Thanks, but I'm not 100% convinced that it is impossible to ever get > into a situation where a domain exists but has no name. In that case, > this patch of yours would leave an undestroyable domain. > > I think the right answer is to use libxl_domain_info to check whether > the domain exists. It returns ERROR_INVAL, without logging anything, > if the domain does not exist. On other failures xl destroy should > probably carry on and attempt to destroy the domain. OK, I looked around the bit, libxl_domain_info really seems to be the easiest solution. So please consider the attached patch. I am not 100% happy with it, actually I think find_domain should return an error value. In this case we could abort the commands gracefully (like xl list does) and avoid the rather impolite exit(2). On the other hand I am not sure if fixing the 30 or so callers of find_domain() is appropriate in this state of development. Please tell me if you prefer this more elaborated version and I will send a patch. Regards, Andre. -- Andre Przywara AMD-Operating System Research Center (OSRC), Dresden, Germany --------------030106090401090509020601 Content-Type: text/plain; name="xl_fix_bad_numeric_id_v2.path" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="xl_fix_bad_numeric_id_v2.path" Content-Description: xl_fix_bad_numeric_id_v2.path diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c index 359970b..deae3f7 100644 --- a/tools/libxl/xl_cmdimpl.c +++ b/tools/libxl/xl_cmdimpl.c @@ -163,12 +163,18 @@ static int cpupool_qualifier_to_cpupoolid(const char *p, uint32_t *poolid_r, static void find_domain(const char *p) { int rc, was_name; + libxl_dominfo info_buf; rc = domain_qualifier_to_domid(p, &domid, &was_name); if (rc) { fprintf(stderr, "%s is an invalid domain identifier (rc=%d)\n", p, rc); exit(2); } + rc = libxl_domain_info(&ctx, &info_buf, domid); + if (rc == ERROR_INVAL) { + fprintf(stderr, "Error: Domain \'%d\' does not exist.\n", domid); + exit(2); + } common_domname = was_name ? p : libxl_domid_to_name(&ctx, domid); } @@ -3149,11 +3155,6 @@ int main_list(int argc, char **argv) } else if (optind == argc-1) { find_domain(argv[optind]); rc = libxl_domain_info(&ctx, &info_buf, domid); - if (rc == ERROR_INVAL) { - fprintf(stderr, "Error: Domain \'%s\' does not exist.\n", - argv[optind]); - return -rc; - } if (rc) { fprintf(stderr, "libxl_domain_info failed (code %d).\n", rc); return -rc; --------------030106090401090509020601 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel --------------030106090401090509020601--