On Mon, May 17, 2010 at 08:41:08AM -0600, Eric Blake wrote:
On 05/17/2010 05:53 AM, Daniel P. Berrange wrote:
@@ -5444,15 +5471,15 @@ static int qemudDomainHotplugVcpus(virDomainObjPtr vm, unsigned int nvcpus) int i, rc; int ret = -1;
+ qemuDomainObjEnterMonitor(vm); + /* We need different branches here, because we want to offline * in reverse order to onlining, so any partial fail leaves us in a * reasonably sensible state */ if (nvcpus > vm->def->vcpus) { for (i = vm->def->vcpus ; i < nvcpus ; i++) { /* Online new CPU */ - qemuDomainObjEnterMonitor(vm); rc = qemuMonitorSetCPU(priv->mon, i, 1); - qemuDomainObjExitMonitor(vm); if (rc == 0) goto unsupported; if (rc < 0) @@ -5463,9 +5490,7 @@ static int qemudDomainHotplugVcpus(virDomainObjPtr vm, unsigned int nvcpus) } else { for (i = vm->def->vcpus - 1 ; i >= nvcpus ; i--) { /* Offline old CPU */ - qemuDomainObjEnterMonitor(vm); rc = qemuMonitorSetCPU(priv->mon, i, 0); - qemuDomainObjExitMonitor(vm); if (rc == 0) goto unsupported; if (rc < 0) @@ -5478,6 +5503,7 @@ static int qemudDomainHotplugVcpus(virDomainObjPtr vm, unsigned int nvcpus) ret = 0;
cleanup: + qemuDomainObjExitMonitor(vm);
This is a larger critical section now, but it didn't look like you were doing anything that had potentially long-running actions that would block such a large critical section.
Actually I've noticed a flaw in this block, not visible in the diff. My change means that we're not holding the mutex when doing vm->def->vcpus++; I'll have to fix this the more verbose way... Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|