All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH][XEND]Fix checkname so that it detects duplicate domains.
@ 2007-05-24  9:57 Petersson, Mats
  2007-05-25  9:20 ` Keir Fraser
  0 siblings, 1 reply; 6+ messages in thread
From: Petersson, Mats @ 2007-05-24  9:57 UTC (permalink / raw)
  To: xen-devel

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

When using "xm restore" or "xm create" it's possible to create multiple
instances of the same domain. 

With this patch, it is not allowed to have multiple instances with the
same name. This should still work for live migration to localhost, as
the migration-path changes the name of the original domain-name, so the
original domain will not name-clash with the migration-target-domain. 

The previous behaviour was to check that the UUID is different, but for
domains that either have UUID in the config file, or that are the result
of restore, the UUID is definitely going to be the same for multiple
instances, so I believe this isn't a correct behaviour in the first
place. 

There is still a bit of a problem that "xm restore" doesn't report the
error correctly - it does say "Restore failed", but not explicitly that
it was caused by using an already existing name.

Signed off by: Mats Petersson (mats.petersson@amd.com)

--
Mats

[-- Attachment #2: patch.fix_checkname --]
[-- Type: application/octet-stream, Size: 621 bytes --]

diff -r 089696e0c603 tools/python/xen/xend/XendDomainInfo.py
--- a/tools/python/xen/xend/XendDomainInfo.py	Thu May 17 11:42:46 2007 +0100
+++ b/tools/python/xen/xend/XendDomainInfo.py	Thu May 24 00:52:30 2007 +0100
@@ -2065,7 +2077,7 @@ class XendDomainInfo:
             raise VmError('Invalid VM Name')
 
         dom =  XendDomain.instance().domain_lookup_nr(name)
-        if dom and dom.info['uuid'] != self.info['uuid']:
+        if dom and dom.domid != self.domid:
             raise VmError("VM name '%s' already exists%s" %
                           (name,
                            dom.domid is not None and

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

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH][XEND]Fix checkname so that it detects duplicate domains.
  2007-05-24  9:57 [PATCH][XEND]Fix checkname so that it detects duplicate domains Petersson, Mats
@ 2007-05-25  9:20 ` Keir Fraser
  2007-05-25  9:25   ` Petersson, Mats
  0 siblings, 1 reply; 6+ messages in thread
From: Keir Fraser @ 2007-05-25  9:20 UTC (permalink / raw)
  To: Petersson, Mats, xen-devel

On 24/5/07 10:57, "Petersson, Mats" <Mats.Petersson@amd.com> wrote:

> The previous behaviour was to check that the UUID is different, but for
> domains that either have UUID in the config file, or that are the result
> of restore, the UUID is definitely going to be the same for multiple
> instances, so I believe this isn't a correct behaviour in the first
> place. 

If we don't enforce UUID uniqueness, what is the point of having a UUID?
Also, don't we store some VM information in /vm/<uuid> in xenstore: how does
that work out if we have multiple domains with the same UUID?

 -- Keir

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH][XEND]Fix checkname so that it detects duplicate domains.
  2007-05-25  9:20 ` Keir Fraser
@ 2007-05-25  9:25   ` Petersson, Mats
  2007-05-25  9:39     ` Keir Fraser
  0 siblings, 1 reply; 6+ messages in thread
From: Petersson, Mats @ 2007-05-25  9:25 UTC (permalink / raw)
  To: Keir Fraser, xen-devel

 

> -----Original Message-----
> From: Keir Fraser [mailto:keir@xensource.com] 
> Sent: 25 May 2007 10:21
> To: Petersson, Mats; xen-devel@lists.xensource.com
> Subject: Re: [Xen-devel] [PATCH][XEND]Fix checkname so that 
> it detects duplicate domains.
> 
> On 24/5/07 10:57, "Petersson, Mats" <Mats.Petersson@amd.com> wrote:
> 
> > The previous behaviour was to check that the UUID is 
> different, but for
> > domains that either have UUID in the config file, or that 
> are the result
> > of restore, the UUID is definitely going to be the same for multiple
> > instances, so I believe this isn't a correct behaviour in the first
> > place. 
> 
> If we don't enforce UUID uniqueness, what is the point of 
> having a UUID?
> Also, don't we store some VM information in /vm/<uuid> in 
> xenstore: how does
> that work out if we have multiple domains with the same UUID?

All very good points. I don't actually know how this is meant to work,
I'm just fixing an apparent bug, which is that if the UUID is
duplicated, you can have two domains with the same name, which isn't
what is supposed to happen. There should probably ALSO be a "_checkuuid"
function to verify that the UUID is unique. Not sure it's my place to
fix that, tho'?

I still think my fix is valid, although, of course, if the UUID is
always guaranteed to be unique even during save/restore/migration within
the same machine, then we could of course use the UUID instead of the
DOMID. 

--
Mats
> 
>  -- Keir
> 
> 
> 
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH][XEND]Fix checkname so that it detects duplicate domains.
  2007-05-25  9:25   ` Petersson, Mats
@ 2007-05-25  9:39     ` Keir Fraser
  2007-05-25  9:44       ` Petersson, Mats
  0 siblings, 1 reply; 6+ messages in thread
From: Keir Fraser @ 2007-05-25  9:39 UTC (permalink / raw)
  To: Petersson, Mats, xen-devel

On 25/5/07 10:25, "Petersson, Mats" <Mats.Petersson@amd.com> wrote:

>> If we don't enforce UUID uniqueness, what is the point of
>> having a UUID?
>> Also, don't we store some VM information in /vm/<uuid> in
>> xenstore: how does
>> that work out if we have multiple domains with the same UUID?
> 
> All very good points. I don't actually know how this is meant to work,
> I'm just fixing an apparent bug, which is that if the UUID is
> duplicated, you can have two domains with the same name, which isn't
> what is supposed to happen. There should probably ALSO be a "_checkuuid"
> function to verify that the UUID is unique. Not sure it's my place to
> fix that, tho'?

Oh. Your patch comment strongly implies that the current behaviour of xend
is to check for UUID uniqueness, and that you are changing this to a check
for name uniqueness. And that looks like what this patch does, too.

But here you seem to be saying that xend enforces neither UUID uniqueness
nor name uniqueness?

 -- Keir

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH][XEND]Fix checkname so that it detects duplicate domains.
  2007-05-25  9:39     ` Keir Fraser
@ 2007-05-25  9:44       ` Petersson, Mats
  2007-05-25  9:51         ` Keir Fraser
  0 siblings, 1 reply; 6+ messages in thread
From: Petersson, Mats @ 2007-05-25  9:44 UTC (permalink / raw)
  To: Keir Fraser, xen-devel

 

> -----Original Message-----
> From: xen-devel-bounces@lists.xensource.com 
> [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of 
> Keir Fraser
> Sent: 25 May 2007 10:40
> To: Petersson, Mats; xen-devel@lists.xensource.com
> Subject: Re: [Xen-devel] [PATCH][XEND]Fix checkname so that 
> it detects duplicate domains.
> 
> On 25/5/07 10:25, "Petersson, Mats" <Mats.Petersson@amd.com> wrote:
> 
> >> If we don't enforce UUID uniqueness, what is the point of
> >> having a UUID?
> >> Also, don't we store some VM information in /vm/<uuid> in
> >> xenstore: how does
> >> that work out if we have multiple domains with the same UUID?
> > 
> > All very good points. I don't actually know how this is 
> meant to work,
> > I'm just fixing an apparent bug, which is that if the UUID is
> > duplicated, you can have two domains with the same name, which isn't
> > what is supposed to happen. There should probably ALSO be a 
> "_checkuuid"
> > function to verify that the UUID is unique. Not sure it's 
> my place to
> > fix that, tho'?
> 
> Oh. Your patch comment strongly implies that the current 
> behaviour of xend
> is to check for UUID uniqueness, and that you are changing 
> this to a check
> for name uniqueness. And that looks like what this patch does, too.
> 
> But here you seem to be saying that xend enforces neither 
> UUID uniqueness
> nor name uniqueness?

That is sort of correct, yes. _IF_ the UUID isn't unique, then the name
also can be duplicated, which doesn't exactly make it any better, right?


I guess there may be a better way to fix this if we know for sure that
UUID's are definitely unique. 

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH][XEND]Fix checkname so that it detects duplicate domains.
  2007-05-25  9:44       ` Petersson, Mats
@ 2007-05-25  9:51         ` Keir Fraser
  0 siblings, 0 replies; 6+ messages in thread
From: Keir Fraser @ 2007-05-25  9:51 UTC (permalink / raw)
  To: Petersson, Mats, xen-devel

On 25/5/07 10:44, "Petersson, Mats" <Mats.Petersson@amd.com> wrote:

> That is sort of correct, yes. _IF_ the UUID isn't unique, then the name
> also can be duplicated, which doesn't exactly make it any better, right?
> 
> I guess there may be a better way to fix this if we know for sure that
> UUID's are definitely unique.

Ah, I see. Okay the patch seems very sensible.

 -- Keir

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2007-05-25  9:51 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-05-24  9:57 [PATCH][XEND]Fix checkname so that it detects duplicate domains Petersson, Mats
2007-05-25  9:20 ` Keir Fraser
2007-05-25  9:25   ` Petersson, Mats
2007-05-25  9:39     ` Keir Fraser
2007-05-25  9:44       ` Petersson, Mats
2007-05-25  9:51         ` Keir Fraser

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.