Re: [libvirt] PATCH: Fix misc locking bugs, and add lock checker

On Thu, May 14, 2009 at 10:19:46PM +0200, Daniel Veillard wrote: It is The QEMU ones are all similar - the virDomainSaveStatus method is taking a 'driver' parameter, meaning we need to keep the lock on 'driver' held for longer. I don't really like this, and think it would be better to remove the driver parameter from this method. We're holding coarse locks for far too long already. Here is an example of what the test reports as error, without the fixes applied. It should help understand the changes I made ================================================================ Function: umlDomainStart ---------------------------------------------------------------- - Total exit points with locked vars: 0 - Total blocks with lock ordering mistakes: 1 - Driver used while unlocked on #line 1564 "uml_driver.c" ret = umlStartVMDaemon(dom->conn, driver___0, vm); ================================================================ ================================================================ Function: umlDomainGetAutostart ---------------------------------------------------------------- - Total exit points with locked vars: 1 - At exit on #line 1675 return (ret); ^^^^^^^^^ variables still locked are | struct uml_driver * driver___0 - Total blocks with lock ordering mistakes: 0 ================================================================ ================================================================ Function: umlDomainSetAutostart ---------------------------------------------------------------- - Total exit points with locked vars: 0 - Total blocks with lock ordering mistakes: 4 - Driver used while unlocked on #line 1704 configFile = virDomainConfigFile(dom->conn, (char const *)driver___0->configDir, (char const *)(vm->def)->name); - Driver used while unlocked on #line 1706 autostartLink = virDomainConfigFile(dom->conn, (char const *)driver___0->autostartDir, (char const *)(vm->def)->name); - Driver used while unlocked on #line 1712 err = virFileMakePath((char const *)driver___0->autostartDir); - Driver used while unlocked on #line 1713 virReportSystemErrorFull(dom->conn, 21, err, "uml_driver.c", "umlDomainSetAutostart", 1715U, (char const *)tmp___1, driver___0->autostartDir); ================================================================ ================================================================ Function: testOpen ---------------------------------------------------------------- - Total exit points with locked vars: 0 - Total blocks with lock ordering mistakes: 3 - Driver used while unlocked on #line 663 "test.c" tmp___20 = virAlloc((void *)(& privconn->domainEventCallbacks), sizeof(*(privconn->domainEventCallbacks))); - Driver used while unlocked on #line 663 privconn->domainEventQueue = virDomainEventQueueNew(); - Driver used while unlocked on #line 670 privconn->domainEventTimer = virEventAddTimeout(-1, & testDomainEventFlush, (void *)privconn, (void (*)(void *opaque ))((void *)0)); ================================================================ ================================================================ Function: qemudClose ---------------------------------------------------------------- - Total exit points with locked vars: 0 - Total blocks with lock ordering mistakes: 1 - Driver used while unlocked on #line 1815 "qemu_driver.c" virDomainEventCallbackListRemoveConn(conn, driver___0->domainEventCallbacks); ================================================================ ================================================================ Function: qemudDomainSuspend ---------------------------------------------------------------- - Total exit points with locked vars: 0 - Total blocks with lock ordering mistakes: 1 - Driver used while unlocked on #line 2259 tmp___4 = qemudSaveDomainStatus(dom->conn, driver___0, vm); ================================================================ ================================================================ Function: qemudDomainResume ---------------------------------------------------------------- - Total exit points with locked vars: 0 - Total blocks with lock ordering mistakes: 1 - Driver used while unlocked on #line 2312 tmp___4 = qemudSaveDomainStatus(dom->conn, driver___0, vm); ================================================================ ================================================================ Function: qemudDomainGetSecurityLabel ---------------------------------------------------------------- - Total exit points with locked vars: 0 - Total blocks with lock ordering mistakes: 1 - Driver used while unlocked on #line 3189 tmp___2 = (*((driver___0->securityDriver)->domainGetSecurityLabel))(dom->conn, vm, seclabel); ================================================================ ================================================================ Function: qemudDomainStart ---------------------------------------------------------------- - Total exit points with locked vars: 0 - Total blocks with lock ordering mistakes: 1 - Driver used while unlocked on #line 3630 ret = qemudStartVMDaemon(dom->conn, driver___0, vm, (char const *)((void *)0), -1); ================================================================ ================================================================ Function: qemudDomainAttachDevice ---------------------------------------------------------------- - Total exit points with locked vars: 0 - Total blocks with lock ordering mistakes: 1 - Driver used while unlocked on #line 4192 tmp___8 = qemudSaveDomainStatus(dom->conn, driver___0, vm); ================================================================ ================================================================ Function: qemudDomainDetachDevice ---------------------------------------------------------------- - Total exit points with locked vars: 0 - Total blocks with lock ordering mistakes: 1 - Driver used while unlocked on #line 4316 tmp___3 = qemudSaveDomainStatus(dom->conn, driver___0, vm); ================================================================ ================================================================ Function: qemudDomainSetAutostart ---------------------------------------------------------------- - Total exit points with locked vars: 0 - Total blocks with lock ordering mistakes: 4 - Driver used while unlocked on #line 4381 configFile = virDomainConfigFile(dom->conn, (char const *)driver___0->configDir, (char const *)(vm->def)->name); - Driver used while unlocked on #line 4383 autostartLink = virDomainConfigFile(dom->conn, (char const *)driver___0->autostartDir, (char const *)(vm->def)->name); - Driver used while unlocked on #line 4389 err = virFileMakePath((char const *)driver___0->autostartDir); - Driver used while unlocked on #line 4390 virReportSystemErrorFull(dom->conn, 10, err, "qemu_driver.c", "qemudDomainSetAutostart", 4392U, (char const *)tmp___1, driver___0->autostartDir); ================================================================ ================================================================ Function: qemudDomainMigratePrepare2 ---------------------------------------------------------------- - Total exit points with locked vars: 0 - Total blocks with lock ordering mistakes: 1 - Object fetched while locked objects exist #line 4990 vm = virDomainAssignDef(dconn, & driver___0->domains, def); ================================================================ ================================================================ Function: storagePoolRefresh ---------------------------------------------------------------- - Total exit points with locked vars: 0 - Total blocks with lock ordering mistakes: 1 - Driver used while unlocked on #line 827 "storage_driver.c" virStoragePoolObjRemove(& driver___0->pools, pool); ================================================================ ================================================================ Function: storagePoolSetAutostart ---------------------------------------------------------------- - Total exit points with locked vars: 0 - Total blocks with lock ordering mistakes: 2 - Driver used while unlocked on #line 962 err = virFileMakePath((char const *)driver___0->autostartDir); - Driver used while unlocked on #line 963 virReportSystemErrorFull(obj->conn, 18, err, "storage_driver.c", "storagePoolSetAutostart", 965U, (char const *)tmp___1, driver___0->autostartDir); ================================================================ ================================================================ Function: storageVolumeCreateXML ---------------------------------------------------------------- - Total exit points with locked vars: 0 - Total blocks with lock ordering mistakes: 1 - Object locked while driver is unlocked on #line 1277 virStoragePoolObjLock(pool); ================================================================ ================================================================ Function: networkStart ---------------------------------------------------------------- - Total exit points with locked vars: 0 - Total blocks with lock ordering mistakes: 1 - Driver used while unlocked on #line 1228 "network_driver.c" ret = networkStartNetworkDaemon(net->conn, driver___0, network); ================================================================ ================================================================ Function: networkDestroy ---------------------------------------------------------------- - Total exit points with locked vars: 0 - Total blocks with lock ordering mistakes: 1 - Driver used while unlocked on #line 1251 ret = networkShutdownNetworkDaemon(net->conn, driver___0, network); ================================================================ ================================================================ Function: networkSetAutostart ---------------------------------------------------------------- - Total exit points with locked vars: 0 - Total blocks with lock ordering mistakes: 4 - Driver used while unlocked on #line 1363 configFile = virNetworkConfigFile(net->conn, (char const *)driver___0->networkConfigDir, (char const *)(network->def)->name); - Driver used while unlocked on #line 1365 autostartLink = virNetworkConfigFile(net->conn, (char const *)driver___0->networkAutostartDir, (char const *)(network->def)->name); - Driver used while unlocked on #line 1371 err = virFileMakePath((char const *)driver___0->networkAutostartDir); - Driver used while unlocked on #line 1372 virReportSystemErrorFull(net->conn, 19, *tmp___1, "network_driver.c", "networkSetAutostart", 1374U, (char const *)tmp___0, driver___0->networkAutostartDir); ================================================================ 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 :|