Re: [libvirt] PATCH: 0/28: Thread safety for libvirtd daemon and drivers

["Daniel P. Berrange" <berrange@xxxxxxxxxx>]

On Wed, Dec 03, 2008 at 04:18:21PM +0100, Daniel Veillard wrote:
> On Mon, Dec 01, 2008 at 12:26:48AM +0000, Daniel P. Berrange wrote:
> > This is a diffstat summary for the combined series of 28 patches
> 
>   Okay, my take at this point is that those should be commited with
> the few fix found my manual examination, maybe extend the documentation
> a bit, and start testing it as much as prossible.
>   Some locking debug facility might be a good addition,

I wrote some an OCaml program using CIL to check driver method exit paths
and validate that all objects were left in an unlocked state. This found
some real bugs !

So here's the incremental fixes for those


diff --git a/src/qemu_driver.c b/src/qemu_driver.c
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -2128,7 +2128,7 @@ static int qemudDomainSave(virDomainPtr 
     if (safewrite(fd, &header, sizeof(header)) != sizeof(header)) {
         qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
                          "%s", _("failed to write save header"));
-        return -1;
+        goto cleanup;
     }
 
     if (safewrite(fd, xml, header.xml_len) != header.xml_len) {
diff --git a/src/test.c b/src/test.c
--- a/src/test.c
+++ b/src/test.c
@@ -2393,7 +2393,7 @@ testStoragePoolCreate(virConnectPtr conn
     }
 
     if (!(pool = virStoragePoolObjAssignDef(conn, &privconn->pools, def))) {
-        return NULL;
+        goto cleanup;
     }
     def = NULL;
 
diff --git a/src/uml_driver.c b/src/uml_driver.c
--- a/src/uml_driver.c
+++ b/src/uml_driver.c
@@ -251,6 +251,7 @@ reread:
 
         if (e->mask & IN_DELETE) {
             if (!virDomainIsActive(dom)) {
+                virDomainObjUnlock(dom);
                 continue;
             }
 
@@ -263,10 +264,12 @@ reread:
             dom->state = VIR_DOMAIN_SHUTOFF;
         } else if (e->mask & (IN_CREATE | IN_MODIFY)) {
             if (virDomainIsActive(dom)) {
+                virDomainObjUnlock(dom);
                 continue;
             }
 
             if (umlReadPidFile(NULL, driver, dom) < 0) {
+                virDomainObjUnlock(dom);
                 continue;
             }
 
@@ -279,6 +282,7 @@ reread:
             if (umlIdentifyChrPTY(NULL, driver, dom) < 0)
                 umlShutdownVMDaemon(NULL, driver, dom);
         }
+        virDomainObjUnlock(dom);
     }
 }
 


Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list