[PATCH]: xl: catch invalid numeric domain ID

[PATCH]: xl: catch invalid numeric domain ID

[Andre Przywara]

[-- Attachment #1: Type: text/plain, Size: 1011 bytes --]

Hi,

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:
# xl destroy 42
libxl: error: libxl.c:692:libxl_domain_destroy xc_domain_pause failed for 42
libxl: error: libxl_dm.c:705:libxl__destroy_device_model Couldn't find 
device model's pid: No such file or directory
libxl: error: libxl.c:696:libxl_domain_destroy 
libxl__destroy_device_model failed for 42
libxl: error: libxl_device.c:325:libxl__devices_destroy 
/local/domain/42/device is empty
libxl: error: libxl_dom.c:536:userdata_path unable to find domain info 
for domain 42: Success
libxl: error: libxl.c:713:libxl_domain_destroy xc_domain_destroy failed 
for 42

The attached patch catches a non-existing domain and returns early (as 
an invalid domain _name_ would do).
Please apply to Xen 4.1.0-rc.

Signed-off-by: Andre Przywara <andre.przywara@amd.com>

-- 
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany

[-- Attachment #2: xl_fix_invalid_numeric_id.patch --]
[-- Type: text/plain, Size: 440 bytes --]

diff -r 003acf02d416 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Thu Jan 20 17:04:06 2011 +0000
+++ b/tools/libxl/xl_cmdimpl.c	Fri Jan 21 17:11:10 2011 +0100
@@ -170,6 +170,10 @@
         exit(2);
     }
     common_domname = was_name ? p : libxl_domid_to_name(&ctx, domid);
+    if (common_domname == NULL) {
+    	fprintf(stderr, "%d is not a valid domain\n", domid);
+    	exit(2);
+    }
 }
 
 static int acquire_lock(void)

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH]: xl: catch invalid numeric domain ID

[Ian Jackson]

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.

Ian.

Re: [PATCH]: xl: catch invalid numeric domain ID

[Andre Przywara]

[-- Attachment #1: Type: text/plain, Size: 1402 bytes --]

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


[-- Attachment #2: xl_fix_bad_numeric_id_v2.path --]
[-- Type: text/plain, Size: 1273 bytes --]

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;

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel