[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[Libvir] PATCH: Fix crash in cleanup when VM creation fails



If using the 'virDomainCreateLinux' call to create a VM, a so called 
'transient' domain will be created - ie one without any config file.
There is special code in the qemudShutdownVMDaemon method to cleanup
the resources associated with such domains, in particuarly free'ing 
the struct qemud_vm  object. Unfortunately in the virDomainCreateLinux
codepath this is a problem, because we still need the 'struct qemud_vm'
object in certain edge cases, and so the caller has to free it. We
currently have a double free() in that scenario. This patch removes
the call to qemudFreeVMDaemon from qemudShutdownVMDaemon. Instead it
is now always the caller's responsibility to cleanup after transient
domains.


 qemu_driver.c |   39 ++++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)


Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 
Index: src/qemu_driver.c
===================================================================
RCS file: /data/cvs/libvirt/src/qemu_driver.c,v
retrieving revision 1.10
diff -u -p -r1.10 qemu_driver.c
--- src/qemu_driver.c	23 Jul 2007 18:00:33 -0000	1.10
+++ src/qemu_driver.c	23 Jul 2007 19:39:07 -0000
@@ -92,9 +92,9 @@ static int qemudStartVMDaemon(virConnect
                               struct qemud_driver *driver,
                               struct qemud_vm *vm);
 
-static int qemudShutdownVMDaemon(virConnectPtr conn,
-                                 struct qemud_driver *driver,
-                                 struct qemud_vm *vm);
+static void qemudShutdownVMDaemon(virConnectPtr conn,
+                                  struct qemud_driver *driver,
+                                  struct qemud_vm *vm);
 
 static int qemudStartNetworkDaemon(virConnectPtr conn,
                                    struct qemud_driver *driver,
@@ -277,6 +277,8 @@ qemudShutdown(void) {
         struct qemud_vm *next = vm->next;
         if (qemudIsActiveVM(vm))
             qemudShutdownVMDaemon(NULL, qemu_driver, vm);
+        if (!vm->configFile[0])
+            qemudRemoveInactiveVM(qemu_driver, vm);
         vm = next;
     }
     
@@ -729,10 +731,10 @@ static int qemudVMData(struct qemud_driv
 }
 
 
-static int qemudShutdownVMDaemon(virConnectPtr conn ATTRIBUTE_UNUSED,
-                                 struct qemud_driver *driver, struct qemud_vm *vm) {
+static void qemudShutdownVMDaemon(virConnectPtr conn ATTRIBUTE_UNUSED,
+                                  struct qemud_driver *driver, struct qemud_vm *vm) {
     if (!qemudIsActiveVM(vm))
-        return 0;
+        return;
 
     qemudLog(QEMUD_INFO, "Shutting down VM '%s'", vm->def->name);
 
@@ -774,24 +776,22 @@ static int qemudShutdownVMDaemon(virConn
 
     driver->nactivevms--;
     driver->ninactivevms++;
-
-    if (!vm->configFile[0])
-        qemudRemoveInactiveVM(driver, vm);
-
-    return 0;
 }
 
 static int qemudDispatchVMLog(struct qemud_driver *driver, struct qemud_vm *vm, int fd) {
-    if (qemudVMData(driver, vm, fd) < 0)
-        if (qemudShutdownVMDaemon(NULL, driver, vm) < 0)
-            return -1;
+    if (qemudVMData(driver, vm, fd) < 0) {
+        qemudShutdownVMDaemon(NULL, driver, vm);
+        if (!vm->configFile[0])
+            qemudRemoveInactiveVM(driver, vm);
+    }
     return 0;
 }
 
 static int qemudDispatchVMFailure(struct qemud_driver *driver, struct qemud_vm *vm,
                                   int fd ATTRIBUTE_UNUSED) {
-    if (qemudShutdownVMDaemon(NULL, driver, vm) < 0)
-        return -1;
+    qemudShutdownVMDaemon(NULL, driver, vm);
+    if (!vm->configFile[0])
+        qemudRemoveInactiveVM(driver, vm);
     return 0;
 }
 
@@ -1844,7 +1844,6 @@ static int qemudDomainResume(virDomainPt
 static int qemudDomainDestroy(virDomainPtr dom) {
     struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData;
     struct qemud_vm *vm = qemudFindVMByID(driver, dom->id);
-    int ret;
 
     if (!vm) {
         qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
@@ -1852,9 +1851,11 @@ static int qemudDomainDestroy(virDomainP
         return -1;
     }
 
-    ret = qemudShutdownVMDaemon(dom->conn, driver, vm);
+    qemudShutdownVMDaemon(dom->conn, driver, vm);
+    if (!vm->configFile[0])
+        qemudRemoveInactiveVM(driver, vm);
     virFreeDomain(dom->conn, dom);
-    return ret;
+    return 0;
 }
 
 

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]