Xen devel - Jul 2008 - [PATCH]xend: fix dual destroy

After changeset 18030 and 18064 were checked in, I found some issues
when creating HVM domains with devices assigned:

In XendDomainInfo.py, we have the call trace: the global function
create() => vm.start()  => _constructDomain().
In _constructDomain(), we invoke xc.test_assign_device() and when the
function fails (maybe because iommu=1 is not specified in grub entry
since iommu is 0 by defaut now; maybe because the device doesn''t exist;
maybe because the device has been assigned, or something) we raise
VmError;
Then start() will invoke self.destroy() and re-raise the exception;
Next, the global create() will invoke vm.destroy() again; finally,
_cleanupVm()  and "self.metrics.destroy()" will be invoked again. Here
when we execute metrics.destroy() for the second time, the deregister()
in XendAPIStore.py will complain: there is no such an element in the
dict __classes and a KeyError exception is raised!

We can avoid the dual destroy by adding a check as follows.

Signed-off-by: Dexuan Cui <dexuan.cui@intel.com>

diff -r f0cd9df566ec tools/python/xen/xend/XendDomainInfo.py
--- a/tools/python/xen/xend/XendDomainInfo.py   Thu Jul 24 17:36:27 2008
+0100
+++ b/tools/python/xen/xend/XendDomainInfo.py   Fri Jul 25 18:00:29 2008
+0800
@@ -2374,6 +2374,9 @@ class XendDomainInfo:
     def destroy(self):
         """Cleanup VM and destroy domain.  Nothrow
guarantee."""

+        if self.domid is None:
+            return
+
         log.debug("XendDomainInfo.destroy: domid=%s",
str(self.domid))

         paths = self._prepare_phantom_paths()

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Cui, Dexuan wrote:
> After changeset 18030 and 18064 were checked in, I found some issues
> when creating HVM domains with devices assigned:
>
> In XendDomainInfo.py, we have the call trace: the global function
> create() => vm.start()  => _constructDomain().
> In _constructDomain(), we invoke xc.test_assign_device() and when the
> function fails (maybe because iommu=1 is not specified in grub entry
> since iommu is 0 by defaut now; maybe because the device doesn''t
exist;
> maybe because the device has been assigned, or something) we raise
> VmError;
> Then start() will invoke self.destroy() and re-raise the exception;
> Next, the global create() will invoke vm.destroy() again; finally,
> _cleanupVm()  and "self.metrics.destroy()" will be invoked again.
Here
> when we execute metrics.destroy() for the second time, the deregister()
> in XendAPIStore.py will complain: there is no such an element in the
> dict __classes and a KeyError exception is raised!
>
> We can avoid the dual destroy by adding a check as follows.
>   
Thanks.  Finding the right place to destroy the metrics object made me 
quite nervous.  Thought I had been through enough testing after c/s 
18064 to have gotten it right :-/.  Even more discouraging - xend still 
has a significant leak.

Jim


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel