Re: [Xen-devel] [PATCH] fixes for suspend & resume

[Chris <hap10@xxxxxxxxxxxxxx>]

Ewan Mellor wrote:
> On Wed, Nov 29, 2006 at 11:22:46AM -0500, Chris wrote:
> 
>> Ewan Mellor wrote:
>>> On Tue, Nov 21, 2006 at 04:45:13PM -0500, Chris wrote:
>>>
>>>> There's at least one additional problem (that this patch doesn't
>>>> address) effecting domains that are started, suspended, resumed, and
>>>> finally shutdown.  Affected domains remain in the xend's list of running
>>>> domains when instead they should revert back to a dormant state with
>>>> domid of -1.  A work around is to restart xend after the effected
>>>> domains have been shutdown, which causes the domains to correctly appear
>>>> as dormant, but clearly this restart shouldn't be necessary.
>>> I believe that this is fixed (or certainly, it's better) with xen-unstable
>>> changeset 12566.
>>>
>>> Ewan.
>>>
>> I think you're right; seems to be fixed from what I can tell.
>>
>> However, I think found another problem. :)  Rebooting a managed domain
>> seems to fail with an exception that the rebooted domain's name already
>> exists.
>>
>> Clearly, the managed domain's name does and should exist to Xend because
>> it's hanging out in the lifecycle area.
>>
>> There's a call to checkName() in XendDomainInfo's __init__() method
>> that's the source of the exception, though I'm debating the merits of
>> where to make changes.  It might be enough to relax checkName() to allow
>> creation of XendDomainInfo instances with non-unique domain names if
>> they only conflict with managed domains that are not running and soon to
>> be replaced.  Although, it would seem more safe if an existing
>> XendDomainInfo instance were re-used when a reboot occurs.
>>
>> Any thoughts?
> 
> Yes, you're right.  I think we're relying upon the fact that the
> XendDomainInfo instances aren't necessarily unique, so I've put a fix in
> comparing the UUIDs of the VMs as specified on that instance.  That should do
> the trick.

I'll be sure test your solution when it appears in the changelog list.

There may be more than one way to skin this cat; for grins here is
another way to approach the name conflict issue.  This patch checks to
see if the domain being rebooted is managed and, if so, calls
XendDomain.domain_start() instead of calling XendDomain.domain_create().
 I think the main advantage is Xend doesn't tend toward creating
non-unique XendDomainInfo instances.  This may not mean much now, but in
the future it might be a useful property, especially if the
XendDomain.domain_lookup() implementation ever changes.  I'll leave that
for you to decide.

That said, don't go replacing the patch you've written with this one
because I hadn't finished working out the kinks yet.  Just consider this
submission food for thought.

Cheers
-Chris

diff -r d1b0a5adaeab tools/python/xen/xend/XendDomainInfo.py
--- a/tools/python/xen/xend/XendDomainInfo.py   Wed Nov 29 23:40:40 2006 +0000
+++ b/tools/python/xen/xend/XendDomainInfo.py   Thu Nov 30 15:51:34 2006 -0500
@@ -981,6 +981,7 @@ class XendDomainInfo:
         False if it is to be destroyed.
         """
         from xen.xend import XendDomain
+        xd = XendDomain.instance()
         
         self._configureBootloader()
         config = self.sxpr()
@@ -997,6 +998,8 @@ class XendDomainInfo:
             return
 
         old_domid = self.domid
+        old_domname = self.info['name']
+        dom_is_managed = xd.is_domain_managed(self)
         self._writeVm(RESTART_IN_PROGRESS, 'True')
 
         now = time.time()
@@ -1029,14 +1032,18 @@ class XendDomainInfo:
 
             new_dom = None
             try:
-                new_dom = XendDomain.instance().domain_create(config)
+                if dom_is_managed:
+                    xd.domain_start(old_domname)
+                    new_dom = xd.domain_lookup(old_domname)
+                else:
+                    new_dom = xd.domain_create(config)
                 new_dom.unpause()
                 rst_cnt = self._readVm('xend/restart_count')
                 rst_cnt = int(rst_cnt) + 1
                 self._writeVm('xend/restart_count', str(rst_cnt))
                 new_dom._removeVm(RESTART_IN_PROGRESS)
             except:
-                if new_dom:
+                if new_dom and not dom_is_managed:
                     new_dom._removeVm(RESTART_IN_PROGRESS)
                     new_dom.destroy()
                 else:

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