[PATCH] Prevent xend from starting duplicate domains

[PATCH] Prevent xend from starting duplicate domains

[Dan Smith]

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

The attached patch puts a simple check in domain_create() which
prevents starting a domain if there is already one of the same name in
the domain list.

This fixes the problem of getting the following error message from xm
while creating a domain soon after destroying one with the same name:

  xen.xend.XendProtocol.XendError: (3, 'No such process')

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


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

diff -r 3a34bcb7c28b tools/python/xen/xend/XendDomain.py
--- a/tools/python/xen/xend/XendDomain.py	Tue Sep 13 19:09:44 2005
+++ b/tools/python/xen/xend/XendDomain.py	Tue Sep 13 13:25:08 2005
@@ -296,6 +296,13 @@
         @param config: configuration
         @return: domain
         """
+
+        existing = self.domains.get_by_name(sxp.child_value(config, "name"))
+        if existing:
+            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] Prevent xend from starting duplicate domains

[David Hopwood]

Dan Smith wrote:
> The attached patch puts a simple check in domain_create() which
> prevents starting a domain if there is already one of the same name in
> the domain list.
> 
> This fixes the problem of getting the following error message from xm
> while creating a domain soon after destroying one with the same name:
> 
>   xen.xend.XendProtocol.XendError: (3, 'No such process')
> 
> Signed-off-by: Dan Smith <danms@us.ibm.com>

Surely there's a race condition here? The hypervisor op that creates a domain
has to fail if it would result in two domains with the same name; this can't
be reliably enforced by a separate check in the Python tools.

-- 
David Hopwood <david.nospam.hopwood@blueyonder.co.uk>

Re: [PATCH] Prevent xend from starting duplicate domains

[Dan Smith]

DH> Surely there's a race condition here? The hypervisor op that
DH> creates a domain has to fail if it would result in two domains
DH> with the same name; 

With the same domid?  Yes.  Unless I'm completely missing something,
the hypervisor knows nothing of the *names* of the domains.

Am I wrong?

DH> this can't be reliably enforced by a separate check in the Python
DH> tools.

Going on the assumption that the names are known only to the tools, I
think the tools are the best place to perform the check.

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

Re: [PATCH] Prevent xend from starting duplicate domains

[Anthony Liguori]

Dan Smith wrote:

>DH> Surely there's a race condition here? The hypervisor op that
>DH> creates a domain has to fail if it would result in two domains
>DH> with the same name; 
>
>With the same domid?  Yes.  Unless I'm completely missing something,
>the hypervisor knows nothing of the *names* of the domains.
>  
>
The hypervisor has no concept of domain names.  That's entirely a Xend 
construct.  The same applies to UUIDs.

Even if there are two domains with the same name, they have different 
domain IDs which is okay from the hypervisor's perspective.

>Am I wrong?
>
>DH> this can't be reliably enforced by a separate check in the Python
>DH> tools.
>
>Going on the assumption that the names are known only to the tools, I
>think the tools are the best place to perform the check.
>
>  
>
Regards,

Anthony Liguori

RE: [PATCH] Prevent xend from starting duplicate domains

[Ian Pratt]

 > The attached patch puts a simple check in domain_create() 
> which prevents starting a domain if there is already one of 
> the same name in the domain list.
> 
> This fixes the problem of getting the following error message 
> from xm while creating a domain soon after destroying one 
> with the same name:
> 
>   xen.xend.XendProtocol.XendError: (3, 'No such process')
> 
> Signed-off-by: Dan Smith <danms@us.ibm.com>

When doing an 'xm destroy' on a domain I wander if xend should rename it
to zombie-<name>?  That way, even if something has gone wrong and we're
left with a shell domain we'll still be able to restart one with the
same name. (obviously we should only do the rename once)

Ian

Re: [PATCH] Prevent xend from starting duplicate domains

[Dan Smith]

IP> When doing an 'xm destroy' on a domain I wander if xend should
IP> rename it to zombie-<name>?  

I thought about doing this exact thing, even putting 'zombie' in the
name :)

IP> That way, even if something has gone wrong and we're left with a
IP> shell domain we'll still be able to restart one with the same
IP> name. (obviously we should only do the rename once)

In xm-test, this problem starts showing up towards the end of the test
run, and increases in frequency.  My guess is that if we were to do
the rename once, it would occasionally allow another domain to be
created, but under high load, would not help that much.

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

Re: [PATCH] Prevent xend from starting duplicate domains

[Christian Limpach]

On 9/14/05, Dan Smith <danms@us.ibm.com> wrote:
> IP> That way, even if something has gone wrong and we're left with a
> IP> shell domain we'll still be able to restart one with the same
> IP> name. (obviously we should only do the rename once)
> 
> In xm-test, this problem starts showing up towards the end of the test
> run, and increases in frequency.  My guess is that if we were to do
> the rename once, it would occasionally allow another domain to be
> created, but under high load, would not help that much.

How about we rename the domain to its uuid[1][2] and don't display
those in xm list unless a verbose option is given?

    christian

[1] <name>-zombie-<domid> could work as well...
[2] or clear the name and change list to supply the uuid if there's no name...

RE: [PATCH] Prevent xend from starting duplicate domains

[Ian Pratt]

> On 9/14/05, Dan Smith <danms@us.ibm.com> wrote:
> > IP> That way, even if something has gone wrong and we're 
> left with a 
> > IP> shell domain we'll still be able to restart one with the same 
> > IP> name. (obviously we should only do the rename once)
> > 
> > In xm-test, this problem starts showing up towards the end 
> of the test 
> > run, and increases in frequency.  My guess is that if we were to do 
> > the rename once, it would occasionally allow another domain to be 
> > created, but under high load, would not help that much.
> 
> How about we rename the domain to its uuid[1][2] and don't 
> display those in xm list unless a verbose option is given?

I think I like the idea of still showing them in 'xm list' just so we
get bug reorts filed.

zombie-domid-name ?

Ian
 
>     christian
> 
> [1] <name>-zombie-<domid> could work as well...
> [2] or clear the name and change list to supply the uuid if 
> there's no name...
> 

Re: [PATCH] Prevent xend from starting duplicate domains

[Dan Smith]

IP> I think I like the idea of still showing them in 'xm list' just so
IP> we get bug reorts filed.

That sounds like a good argument to me.

IP> zombie-domid-name ?

I'll put this on my to-do list.

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

Re: [PATCH] Prevent xend from starting duplicate domains

[Christian Limpach]

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

On 9/13/05, Dan Smith <danms@us.ibm.com> wrote:
> The attached patch puts a simple check in domain_create() which
> prevents starting a domain if there is already one of the same name in
> the domain list.
> 
> This fixes the problem of getting the following error message from xm
> while creating a domain soon after destroying one with the same name:
> 
>  xen.xend.XendProtocol.XendError: (3, 'No such process')

There is already a check for domains with the same name in
XendDomainInfo.check_name.  We should fix that check instead of adding
another check.  Could you try the attached patch which removes the
check if a domain "is terminated" and thus allows creation of a domain
with the same name?

It's arguable if this should be allowed or not, but if we allow it,
then we need to find the root cause of the error you're seeing...

    christian

[-- Attachment #2: fix-name-check.patch --]
[-- Type: application/octet-stream, Size: 474 bytes --]

diff -r 4490e39fc322 tools/python/xen/xend/XendDomainInfo.py
--- a/tools/python/xen/xend/XendDomainInfo.py	Fri Sep 16 19:11:39 2005
+++ b/tools/python/xen/xend/XendDomainInfo.py	Fri Sep 16 20:40:23 2005
@@ -635,8 +635,6 @@
         # my domain id.
         if not dominfo:
             return
-        if dominfo.is_terminated():
-            return
         if not self.domid or (dominfo.domid != self.domid):
             raise VmError('vm name clash: ' + name)
         

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

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

Re: [PATCH] Prevent xend from starting duplicate domains

[Dan Smith]

CL> There is already a check for domains with the same name in
CL> XendDomainInfo.check_name.

Ah, yes, I see that now.

CL> We should fix that check instead of adding another check.

I agree that there should be one check, but it seems counter-intuitive
(to me) to have that check where it is.  That's the reason I hadn't
noticed it before.

It seems strange to have a container class (XendDomain) that
instantiates an item object which then gets an instance of the
container to check for another domain with the same name.  Why not
have the container itself do the duplicate check?  I would argue that
the existing model is bad because the item class could not be placed
in another container.  Further, the container is enforced as a set by
the items that go in it, instead of the container itself.

If others agree, I'd be happy to submit a patch that moves the check
out of the item class and into the container class.

CL> Could you try the attached patch which removes the check if a
CL> domain "is terminated" and thus allows creation of a domain with
CL> the same name?

I did test the patch and it does prevent corruption of the list.

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

Re: [PATCH] Prevent xend from starting duplicate domains

[Christian Limpach]

On 9/16/05, Dan Smith <danms@us.ibm.com> wrote:
> CL> We should fix that check instead of adding another check.
> 
> I agree that there should be one check, but it seems counter-intuitive
> (to me) to have that check where it is.  That's the reason I hadn't
> noticed it before.
> 
> It seems strange to have a container class (XendDomain) that
> instantiates an item object which then gets an instance of the
> container to check for another domain with the same name.  Why not
> have the container itself do the duplicate check?  I would argue that
> the existing model is bad because the item class could not be placed
> in another container.  Further, the container is enforced as a set by
> the items that go in it, instead of the container itself.
> 
> If others agree, I'd be happy to submit a patch that moves the check
> out of the item class and into the container class.

I agree, go for it.  You'll have to check both in the create and
restore cases.  I don't think we need to check on recreate, we can
assume a consistent store...

> CL> Could you try the attached patch which removes the check if a
> CL> domain "is terminated" and thus allows creation of a domain with
> CL> the same name?
> 
> I did test the patch and it does prevent corruption of the list.

I'm happy with removing the check, but I wonder if we're not masking
the fact that we set the domain to terminated state when it's not yet
in that state.  Yeah, looks like we should only set the state to
terminated once we've completed device cleanup.
Of course we must have some other bug which you hit when we have
domains in terminated state and you want to create a new domain of the
same name.  Maybe renaming the domain when we set the terminated state
is the solution for that...

    christian