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.
_______________________________________________
vdsm-devel mailing list
vdsm-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel

Reply via email to