[PATCH] Make xend reject duplicates and rename zombies

[PATCH] Make xend reject duplicates and rename zombies

[Dan Smith]

[-- Attachment #1: Type: text/plain, Size: 445 bytes --]

This patch is an update of my previous anti-duplicate-domain patch.
Now, we check an existing same-name domain to see if it's in the
"terminated" state, renaming it to "zombie-domid-name" if so.

This basically makes the problem go away for me, as it gives the dying
domain time to clean itself up.

Test 10_create_fastdestroy from the next release of xm-test validates
that this fixes the problem.

Signed-off-by: Dan Smith <danms@us.ibm.com>


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: xend-dup-updated.patch --]
[-- Type: text/x-patch, Size: 1026 bytes --]

diff -r c27431cf81f9 tools/python/xen/xend/XendDomain.py
--- a/tools/python/xen/xend/XendDomain.py	Thu Sep 15 13:17:24 2005
+++ b/tools/python/xen/xend/XendDomain.py	Thu Sep 15 08:44:22 2005
@@ -297,6 +297,20 @@
         @param config: configuration
         @return: domain
         """
+
+        existing = self.domains.get_by_name(sxp.child_value(config, "name"))
+        if existing:
+            if existing.is_terminated():
+                newname = "zombie-%i-%s" % (existing.domid, existing.name)
+                log.debug("Renaming zombie domain %s -> %s" %
+                          (existing.name, newname))
+                existing.setName(newname)
+            else:
+                log.debug("Attempt to create duplicate domain %s" %
+                          existing.name)
+                raise XendError("Domain %s already exists as %i!" %
+                                (existing.name, existing.id))
+        
         dominfo = XendDomainInfo.create(self.dbmap, config)
         return dominfo
 

[-- Attachment #3: Type: text/plain, Size: 88 bytes --]


-- 
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms@us.ibm.com

[-- Attachment #4: Type: text/plain, Size: 138 bytes --]

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

Re: [PATCH] Make xend reject duplicates and rename zombies

[Anthony Liguori]

I think this is not the right solution to the problem at hand.  The 
problem stems from the fact that xm destroy is just a call to 
xc_domain_destroy which is really just a request to the hypervisor to 
destroy the domain.

Therefore, there is a race condition if you assume that the domain is 
dead after xm destroy returns.  This patch renames the domain name which 
prevents a name class but does not solve the general problem.  Consider, 
for instance, if a domain is using a block device and you do an xm 
destroy.  It is not safe to create a new domain with that same block 
device until you know that the previously mentioned domain is gone.

This patch would allow:

xm destroy xmexample1 && xm create /etc/xen/xmexample1

Even though it might really be conflicting.  This could lead to *very* 
subtle device corruption down the road.

I think the right solution is to make xm destroy not return until the 
domain has actually gone away and add a flag to xm destroy to return 
immediately if that behavior is ever desired.

I'll work up a patch tonight after class.

Regards,

Anthony Liguori

Dan Smith wrote:

>This patch is an update of my previous anti-duplicate-domain patch.
>Now, we check an existing same-name domain to see if it's in the
>"terminated" state, renaming it to "zombie-domid-name" if so.
>
>This basically makes the problem go away for me, as it gives the dying
>domain time to clean itself up.
>
>Test 10_create_fastdestroy from the next release of xm-test validates
>that this fixes the problem.
>
>Signed-off-by: Dan Smith <danms@us.ibm.com>
>
>  
>
>------------------------------------------------------------------------
>
>diff -r c27431cf81f9 tools/python/xen/xend/XendDomain.py
>--- a/tools/python/xen/xend/XendDomain.py	Thu Sep 15 13:17:24 2005
>+++ b/tools/python/xen/xend/XendDomain.py	Thu Sep 15 08:44:22 2005
>@@ -297,6 +297,20 @@
>         @param config: configuration
>         @return: domain
>         """
>+
>+        existing = self.domains.get_by_name(sxp.child_value(config, "name"))
>+        if existing:
>+            if existing.is_terminated():
>+                newname = "zombie-%i-%s" % (existing.domid, existing.name)
>+                log.debug("Renaming zombie domain %s -> %s" %
>+                          (existing.name, newname))
>+                existing.setName(newname)
>+            else:
>+                log.debug("Attempt to create duplicate domain %s" %
>+                          existing.name)
>+                raise XendError("Domain %s already exists as %i!" %
>+                                (existing.name, existing.id))
>+        
>         dominfo = XendDomainInfo.create(self.dbmap, config)
>         return dominfo
> 
>  
>
>------------------------------------------------------------------------
>
>
>  
>
>------------------------------------------------------------------------
>
>_______________________________________________
>Xen-devel mailing list
>Xen-devel@lists.xensource.com
>http://lists.xensource.com/xen-devel
>  
>

Re: [PATCH] Make xend reject duplicates and rename zombies

[Dan Smith]

AL> This patch renames the domain name which prevents a name class but
AL> does not solve the general problem.

I agree.  This is why in my original patch, I simply had the create
routine reject attempts to create domains with duplicate names.

Further, I will point out that what I meant by my comment attached to
this patch was not that it solved the general problem, but that it
seemed to make the problem not get in the way of some of my existing
tests :)

AL> I think the right solution is to make xm destroy not return until
AL> the domain has actually gone away and add a flag to xm destroy to
AL> return immediately if that behavior is ever desired.

Yep, I think that's a much better solution.  So, I think my original
patch should still be applied: xend should do its own checking for
duplicates, so that we don't rely on whatever tool is asking us to do
so.

-- 
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms@us.ibm.com

Re: [PATCH] Make xend reject duplicates and rename zombies

[Ted Kaczmarek]

On Thu, 2005-09-15 at 12:16 -0700, Dan Smith wrote:
> AL> This patch renames the domain name which prevents a name class but
> AL> does not solve the general problem.
> 
> I agree.  This is why in my original patch, I simply had the create
> routine reject attempts to create domains with duplicate names.
> 
> Further, I will point out that what I meant by my comment attached to
> this patch was not that it solved the general problem, but that it
> seemed to make the problem not get in the way of some of my existing
> tests :)
> 
> AL> I think the right solution is to make xm destroy not return until
> AL> the domain has actually gone away and add a flag to xm destroy to
> AL> return immediately if that behavior is ever desired.
> 
> Yep, I think that's a much better solution.  So, I think my original
> patch should still be applied: xend should do its own checking for
> duplicates, so that we don't rely on whatever tool is asking us to do
> so.
> 
Has this been applied yet?

I still have this exact issue with change set 6884.

Regards,
Ted

Re: [Xen-tools] Re: [PATCH] Make xend reject duplicates and rename zombies

[Christian Limpach]

On 9/15/05, Anthony Liguori <aliguori@us.ibm.com> wrote:
> I think this is not the right solution to the problem at hand.  The
> problem stems from the fact that xm destroy is just a call to
> xc_domain_destroy which is really just a request to the hypervisor to
> destroy the domain.

Indeed.

> Therefore, there is a race condition if you assume that the domain is
> dead after xm destroy returns.  This patch renames the domain name which
> prevents a name class but does not solve the general problem.  Consider,
> for instance, if a domain is using a block device and you do an xm
> destroy.  It is not safe to create a new domain with that same block
> device until you know that the previously mentioned domain is gone.

I think renaming would make sense if a domain is crashed.  You might
want to keep the domain around to attach a debugger to it but at the
same time restart a fresh copy as soon as possible.

> This patch would allow:
> xm destroy xmexample1 && xm create /etc/xen/xmexample1
> 
> I think the right solution is to make xm destroy not return until the
> domain has actually gone away and add a flag to xm destroy to return
> immediately if that behavior is ever desired.

Sounds good.

> I'll work up a patch tonight after class.

Thanks!

     christian

Re: [Xen-tools] Re: [PATCH] Make xend reject duplicates and rename zombies

[Anthony Liguori]

Christian Limpach wrote:

>On 9/15/05, Anthony Liguori <aliguori@us.ibm.com> wrote:
>  
>
>>I think this is not the right solution to the problem at hand.  The
>>problem stems from the fact that xm destroy is just a call to
>>xc_domain_destroy which is really just a request to the hypervisor to
>>destroy the domain.
>>    
>>
>
>Indeed.
>
>  
>
>>Therefore, there is a race condition if you assume that the domain is
>>dead after xm destroy returns.  This patch renames the domain name which
>>prevents a name class but does not solve the general problem.  Consider,
>>for instance, if a domain is using a block device and you do an xm
>>destroy.  It is not safe to create a new domain with that same block
>>device until you know that the previously mentioned domain is gone.
>>    
>>
>
>I think renaming would make sense if a domain is crashed.  You might
>want to keep the domain around to attach a debugger to it but at the
>same time restart a fresh copy as soon as possible.
>  
>
As long as you can make sure to rename *after* all of the devices have 
been properly torn down.

Otherwise, we need to make sure to make it well known that restarting a 
domain after a crash can result in very bad things :-)

Regards,

Anthony Liguori

>>This patch would allow:
>>xm destroy xmexample1 && xm create /etc/xen/xmexample1
>>
>>I think the right solution is to make xm destroy not return until the
>>domain has actually gone away and add a flag to xm destroy to return
>>immediately if that behavior is ever desired.
>>    
>>
>
>Sounds good.
>
>  
>
>>I'll work up a patch tonight after class.
>>    
>>
>
>Thanks!
>
>     christian
>
>  
>

Re: [Xen-tools] Re: [PATCH] Make xend reject duplicates and rename zombies

[Christian Limpach]

On 9/16/05, Anthony Liguori <aliguori@us.ibm.com> wrote:
> >I think renaming would make sense if a domain is crashed.  You might
> >want to keep the domain around to attach a debugger to it but at the
> >same time restart a fresh copy as soon as possible.
>
> As long as you can make sure to rename *after* all of the devices have
> been properly torn down.
> 
> Otherwise, we need to make sure to make it well known that restarting a
> domain after a crash can result in very bad things :-)

How so?  The crashed domain is certainly not going to do anything with
those devices anymore...  Actually, I'm not even sure I buy the
"unsafe to reuse devices" argument if the domain is only hanging
around in an almost dead state -- it's almost dead, it's not going to
do any requests on those devices anymore...

So regarding the check for termination in check_name, if we only
switch the state to terminated once the domain is almost dead, reusing
the name should be fine...

    christian

Re: [Xen-tools] Re: [PATCH] Make xend reject duplicates and rename zombies

[Anthony Liguori]

Christian Limpach wrote:

>On 9/16/05, Anthony Liguori <aliguori@us.ibm.com> wrote:
>  
>
>>>I think renaming would make sense if a domain is crashed.  You might
>>>want to keep the domain around to attach a debugger to it but at the
>>>same time restart a fresh copy as soon as possible.
>>>      
>>>
>>As long as you can make sure to rename *after* all of the devices have
>>been properly torn down.
>>
>>Otherwise, we need to make sure to make it well known that restarting a
>>domain after a crash can result in very bad things :-)
>>    
>>
>
>How so?  The crashed domain is certainly not going to do anything with
>those devices anymore...  Actually, I'm not even sure I buy the
>"unsafe to reuse devices" argument if the domain is only hanging
>around in an almost dead state -- it's almost dead, it's not going to
>do any requests on those devices anymore...
>  
>
I'm thinking specifically about block devices.  While the domain may 
have died, there may be requests in the block devices queues that have 
not been flushed to disk yet.  This is what would lead to problems..

Regards,

Anthony Liguori

>So regarding the check for termination in check_name, if we only
>switch the state to terminated once the domain is almost dead, reusing
>the name should be fine...
>
>    christian
>
>  
>

Re: [Xen-tools] Re: [PATCH] Make xend reject duplicates and rename zombies

[Christian Limpach]

On 9/16/05, Anthony Liguori <aliguori@us.ibm.com> wrote:
> I'm thinking specifically about block devices.  While the domain may
> have died, there may be requests in the block devices queues that have
> not been flushed to disk yet.  This is what would lead to problems..

sure, but the logic which prevents you from using the same block
device multiple times should kick in at this point -- granted, that
code's a bit broken right now...

    christian