On Thu, Aug 01, 2013 at 05:31:44AM -0400, Antoni Segura Puimedon wrote:
Looking at clientIF.py on the patch Michal pointed to, concretely on:
- def removeVmFromMonitoredDomains(self, vmId):
for dom in self.domainVmIds:
#only take lock here to allow runVm to take the lock in-between
with self.domainVmIdsLock:
try:
self.domainVmIds[dom].remove(vmId)
except ValueError:
pass
else:
if not self.domainVmIds[dom]:
del self.domainVmIds[dom]
If there is no exception we will be deleting an element from a dictionary while iterating it. This is dangerous and should be avoided. The fastest alternative to implement would be that in the "else" we append dom to a list of domainsToRemove and after the iteration we get a lock and remove all of them.
Alternatively, I believe we could do without maintaining the domain->vmId mapping. I did not understand why having each VM hold its domains is not enough.
The said patch has another problem - a single SIGSTOPped qemu process can block all SPM operations (in case its storage domain has become active again and we attempt to cont the VM).
Dan.