Ensure domains are killed off if hotplug fails. Fix deletion of inactive domains

diff --git a/ChangeLog b/ChangeLog
index cba4748a26a2e9a9c2138db5e4084f00513067b6..5296f8b88b3013457b2c975d2ea290763c45a8ef 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+Thu Feb 22 14:06:24 EST 2007 Daniel P. Berrange <berrange@redhat.com>
+
+       * src/xm_internal.c, src/xend_internal.c: Re-arrange VM creation
+       commands to ensure we destroy stillborn domains if device hotplug
+       fails to complete. Fix deletion of inactive VMs from internal
+       cache of domain configs.
+
 Thu Feb 22 11:45:24 EST 2007 Daniel P. Berrange <berrange@redhat.com>
 
        * src/xs_internal.c: Refuse to do shutdown / reboot on

diff --git a/src/xend_internal.c b/src/xend_internal.c
index d78cbba2e31d14e9a0e4d011dbec5db3a71d008b..73325725c457b47038693436bd15ecfb9993d676 100644 (file)
--- a/src/xend_internal.c
+++ b/src/xend_internal.c
@@ -2816,7 +2816,7 @@ xenDaemonCreateLinux(virConnectPtr conn, const char *xmlDesc,
     int ret;
     char *sexpr;
     char *name = NULL;
-    virDomainPtr dom;
+    virDomainPtr dom = NULL;
 
     if (!VIR_IS_CONNECT(conn)) {
         virXendError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
@@ -2841,32 +2841,30 @@ xenDaemonCreateLinux(virConnectPtr conn, const char *xmlDesc,
     ret = xenDaemonDomainCreateLinux(conn, sexpr);
     free(sexpr);
     if (ret != 0) {
-        fprintf(stderr, _("Failed to create domain %s\n"), name);
         goto error;
     }
 
-    ret = xend_wait_for_devices(conn, name);
-    if (ret != 0) {
-        fprintf(stderr, _("Failed to get devices for domain %s\n"), name);
+    /* This comes before wait_for_devices, to ensure that latter
+       cleanup will destroy the domain upon failure */
+    if (!(dom = virDomainLookupByName(conn, name)))
         goto error;
-    }
 
-    dom = virDomainLookupByName(conn, name);
-    if (dom == NULL) {
+    if ((ret = xend_wait_for_devices(conn, name)) < 0)
         goto error;
-    }
 
-    ret = xenDaemonDomainResume(dom);
-    if (ret != 0) {
-        fprintf(stderr, _("Failed to resume new domain %s\n"), name);
-        xenDaemonDomainDestroy(dom);
+    if ((ret = xenDaemonDomainResume(dom)) < 0)
         goto error;
-    }
 
     free(name);
 
     return (dom);
+
   error:
+    /* Make sure we don't leave a still-born domain around */
+    if (dom != NULL) {
+        xenDaemonDomainDestroy(dom);
+        virFreeDomain(dom->conn, dom);
+    }
     if (name != NULL)
         free(name);
     return (NULL);

diff --git a/src/xm_internal.c b/src/xm_internal.c
index 2e1d88e7061ddeb7fc10b11eee9dd572bcc0518e..f2b8a1a5452b935d0f4cdb9c08c578944fed86f5 100644 (file)
--- a/src/xm_internal.c
+++ b/src/xm_internal.c
@@ -50,7 +50,9 @@ typedef struct xenXMConfCache {
 } xenXMConfCache;
 
 static char configDir[PATH_MAX];
+/* Config file name to config object */
 static virHashTablePtr configCache = NULL;
+/* Name to config file name */
 static virHashTablePtr nameConfigMap = NULL;
 static int nconnections = 0;
 static time_t lastRefresh = 0;
@@ -1295,13 +1297,6 @@ int xenXMDomainCreate(virDomainPtr domain) {
     ret = xenDaemonDomainCreateLinux(domain->conn, sexpr);
     free(sexpr);
     if (ret != 0) {
-        fprintf(stderr, "Failed to create domain %s\n", domain->name);
-        return (-1);
-    }
-
-    ret = xend_wait_for_devices(domain->conn, domain->name);
-    if (ret != 0) {
-        fprintf(stderr, "Failed to get devices for domain %s\n", domain->name);
         return (-1);
     }
 
@@ -1310,15 +1305,20 @@ int xenXMDomainCreate(virDomainPtr domain) {
     }
     domain->id = ret;
 
-    ret = xenDaemonDomainResume(domain);
-    if (ret != 0) {
-        fprintf(stderr, "Failed to resume new domain %s\n", domain->name);
+    if ((ret = xend_wait_for_devices(domain->conn, domain->name)) < 0)
+        goto cleanup;
+
+    if ((ret = xenDaemonDomainResume(domain)) < 0)
+        goto cleanup;
+
+    return (0);
+
+ cleanup:
+    if (domain->id != -1) {
         xenDaemonDomainDestroy(domain);
         domain->id = -1;
-        return (-1);
     }
-
-    return (0);
+    return (-1);
 }
 
 
@@ -2165,10 +2165,12 @@ int xenXMDomainUndefine(virDomainPtr domain) {
     if (unlink(entry->filename) < 0)
         return (-1);
 
-    if (virHashRemoveEntry(nameConfigMap, entry->filename, NULL) < 0)
+    /* Remove the name -> filename mapping */
+    if (virHashRemoveEntry(nameConfigMap, domain->name, NULL) < 0)
         return(-1);
 
-    if (virHashRemoveEntry(configCache, domain->name, xenXMConfigFree) < 0)
+    /* Remove the config record itself */
+    if (virHashRemoveEntry(configCache, entry->filename, xenXMConfigFree) < 0)
         return (-1);
 
     return (0);