Xen devel - Sep 2005 - [PATCH] Prevent xend from starting duplicate domains

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>






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

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

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>


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

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


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

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

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

> 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

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

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


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

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...

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

> 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...
> 
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

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


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

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


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

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


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

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

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