Re: PATCH: Fix name uniqueness check

["Daniel P. Berrange" <berrange@redhat.com>]

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

On Mon, Oct 01, 2007 at 07:18:11PM +0100, Daniel P. Berrange wrote:
> There are 6 basic test cases:
> 
>    a. same name + same UUID:
>    b. diff name + same UUID:
>    c. same name + diff UUID:
>    d. diff name + diff UUID:
>    e. same name + no UUID:
>    f. diff name + no UUID:
> 
> These need to be tested with 'xm create' and 'xm new', and all tests need to
> be done with the pre-existing VM both inactive, and active.

So, to 6 tests under 4 scenarios, gives 24 combinations. NB in the list
above from my previous mail I flipped some letters compared to my actual
testsuite - so pay attention to the letters used below, not the ones above.

UUID is the primary unique key for VMs. Name is the secondary unique
key. The former takes priority, eg redefining with existing UUID, but
different name causes a rename in the config stored.

  - Scenario 1: xm new + existing inactive vm

      a. same name + same UUID:  ALLOW  (redefine)
      b. same name + diff UUID:  REJECT (clashing name)
      c. diff name + same UUID:  ALLOW  (redefine+rename)
      d. diff name + diff UUID:  ALLOW  (define)
      e. same name + no UUID:    REJECT (clashing name)
      f. diff name + no UUID:    ALLOW  (define)

  - Scenario 2: xm create + existing inactive vm

      a. same name + same UUID:  ALLOW  (create)
      b. same name + diff UUID:  REJECT (clashing name)
      c. diff name + same UUID:  ALLOW  (create)
      d. diff name + diff UUID:  ALLOW  (create)
      e. same name + no UUID:    REJECT (clashing name)
      f. diff name + no UUID:    ALLOW  (create)

  - Scenario 3: xm new + existing active vm

      a. same name + same UUID:  ALLOW  (redefine)
      b. same name + diff UUID:  REJECT (clashing name)
      c. diff name + same UUID:  ALLOW  (redefine+rename)
      d. diff name + diff UUID:  ALLOW  (define)
      e. same name + no UUID:    REJECT (clashing name)
      f. diff name + no UUID:    ALLOW  (define)

  - Scenario 4: xm create + existing active vm

      a. same name + same UUID:  REJECT (clashing name/uuid)
      b. same name + diff UUID:  REJECT (clashing name)
      c. diff name + same UUID:  REJECT (clashing uuid)
      d. diff name + diff UUID:  REJECT (disk image in use)
      e. same name + no UUID:    REJECT (clashing name)
      f. diff name + no UUID:    REJECT (disk image in use)

Basically the allow/reject rules at the same for the first 3 scenarios.
The reject scnarios are dealt with by the _checkName() function in the
XendDomainInfo class. Only in the last one do we have to reject all combos
more aggressively. This is because if you 'xm create' the same VM with 
identical details it wouldn't be stopped by the hotplug disk checks in 
all cases. This is not possible to validate from _checkName() since it 
does not have enough context to know that the new VM is about to be started.
So we have to make extra checks in the 'create()' function in XendDomainInfo

I am attaching a file 'test.sh' which creates a test VM & configs and runs
through all these 24 scenarios. Feel free to add this to XenD's regression
test suite if practical.

So the actual patches needed...

In 3.1-testing

 - Revert 15168:a717cb2fac90  (aka pull in 15973:8817a53c030f from 
   xen-unstable.hg). This addresses scenarios 1-3 above

 - Apply the 'xen-create-name-uniqueness.patch'  which adds a check
   to XendDomainInfo.create() to deal with special needs of scenario 4

In xen-unstable

 - Revert 15642:207582c8d88b (this patch is now bogus since 15168:a717cb2fac90
   is already reverted by 15973:8817a53c030f). It only covered 'xm new'
   where as proposed patches cover 'xm new' and 'xm create' together

 - Apply the 'xen-create-name-uniqueness.patch'  which adds a check
   to XendDomainInfo.create() to deal with special needs of scenario 4

I don't have a xen-unstable.hg box currently, so my testing of these patches
has been against the 3.1-testing.hg tree. The 'test.sh' script should be able
to validate them for unstable. BTW, run test.sh with /etc/xen as CWD.

  Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

Regards,
Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 

[-- Attachment #2: xen-create-name-uniqueness.patch --]
[-- Type: text/plain, Size: 1066 bytes --]

diff -r 687ce253a0f4 tools/python/xen/xend/XendDomainInfo.py
--- a/tools/python/xen/xend/XendDomainInfo.py	Wed Sep 26 10:05:02 2007 +0100
+++ b/tools/python/xen/xend/XendDomainInfo.py	Mon Oct 01 17:07:43 2007 -0400
@@ -74,9 +74,15 @@ def create(config):
     @return: An up and running XendDomainInfo instance
     @raise VmError: Invalid configuration or failure to start.
     """
-
+    from xen.xend import XendDomain
+    domconfig = XendConfig.XendConfig(sxp_obj = config)
+    othervm = XendDomain.instance().domain_lookup_nr(domconfig["name_label"])
+    if othervm is None or othervm.domid is None:
+        othervm = XendDomain.instance().domain_lookup_nr(domconfig["uuid"])
+    if othervm is not None and othervm.domid is not None:
+        raise VmError("Domain '%s' already exists with ID '%d'" % (domconfig["name_label"], othervm.domid))
     log.debug("XendDomainInfo.create(%s)", scrub_password(config))
-    vm = XendDomainInfo(XendConfig.XendConfig(sxp_obj = config))
+    vm = XendDomainInfo(domconfig)
     try:
         vm.start()
     except:

[-- Attachment #3: test.sh --]
[-- Type: application/x-sh, Size: 3905 bytes --]

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

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