[Xen-devel] Re: [PATCH, v2]: xl: Check domain existance when doing domain identifier lookups

[Gianni Tedesco <gianni.tedesco@xxxxxxxxxx>]

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 <gianni.tedesco@xxxxxxxxxx>

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)



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