From 651cc3e2c8848a3e299d59a9b3801f3ac7d6c678 Mon Sep 17 00:00:00 2001
From: Christos Stavrakakis <cstavr@grnet.gr>
Date: Thu, 4 Apr 2013 11:51:10 +0300
Subject: [PATCH] Check that device names are unique and valid

Extend the CheckArguments phase of LUInstanceCreate and CheckPrereq
phase of LUInstanceSetParams to also check if the name parameters of
disks and NICs are unique and valid.

Signed-off-by: Christos Stavrakakis <cstavr@grnet.gr>
Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>
Reviewed-by: Helga Velroyen <helgav@google.com>
---
 lib/cmdlib.py                    |   16 ++++++++++++--
 lib/utils/__init__.py            |   44 ++++++++++++++++++++++++++++++++++++++
 test/py/ganeti.utils_unittest.py |   21 ++++++++++++++++++
 3 files changed, 79 insertions(+), 2 deletions(-)

diff --git a/lib/cmdlib.py b/lib/cmdlib.py
index 9e09583..1995b2b 100644
--- a/lib/cmdlib.py
+++ b/lib/cmdlib.py
@@ -10186,6 +10186,11 @@ class LUInstanceCreate(LogicalUnit):
     # check nics' parameter names
     for nic in self.op.nics:
       utils.ForceDictType(nic, constants.INIC_PARAMS_TYPES)
+    # check that NIC's parameters names are unique and valid
+    utils.ValidateDeviceNames("NIC", self.op.nics)
+
+    # check that disk's names are unique and valid
+    utils.ValidateDeviceNames("disk", self.op.disks)
 
     cluster = self.cfg.GetClusterInfo()
     if not self.op.disk_template in cluster.enabled_disk_templates:
@@ -14009,9 +14014,14 @@ class LUInstanceSetParams(LogicalUnit):
                                  " (%d), cannot add more" % constants.MAX_NICS,
                                  errors.ECODE_STATE)
 
+    def _PrepareDiskMod(_, disk, params, __):
+      disk.name = params.get(constants.IDISK_NAME, None)
+
     # Verify disk changes (operating on a copy)
-    disks = instance.disks[:]
-    ApplyContainerMods("disk", disks, None, self.diskmod, None, None, None)
+    disks = copy.deepcopy(instance.disks)
+    ApplyContainerMods("disk", disks, None, self.diskmod, None, _PrepareDiskMod,
+                       None)
+    utils.ValidateDeviceNames("disk", disks)
     if len(disks) > constants.MAX_DISKS:
       raise errors.OpPrereqError("Instance has too many disks (%d), cannot add"
                                  " more" % constants.MAX_DISKS,
@@ -14033,6 +14043,8 @@ class LUInstanceSetParams(LogicalUnit):
       nics = [nic.Copy() for nic in instance.nics]
       ApplyContainerMods("NIC", nics, self._nic_chgdesc, self.nicmod,
                          self._CreateNewNic, self._ApplyNicMods, None)
+      # Verify that NIC names are unique and valid
+      utils.ValidateDeviceNames("NIC", nics)
       self._new_nics = nics
       ispec[constants.ISPEC_NIC_COUNT] = len(self._new_nics)
     else:
diff --git a/lib/utils/__init__.py b/lib/utils/__init__.py
index d3319c1..cda87f4 100644
--- a/lib/utils/__init__.py
+++ b/lib/utils/__init__.py
@@ -797,3 +797,47 @@ class FieldSet(object):
 
     """
     return [val for val in items if not self.Matches(val)]
+
+
+def ValidateDeviceNames(kind, container):
+  """Validate instance device names.
+
+  Check that a device container contains only unique and valid names.
+
+  @type kind: string
+  @param kind: One-word item description
+  @type container: list
+  @param container: Container containing the devices
+
+  """
+
+  valid = []
+  for device in container:
+    if isinstance(device, dict):
+      if kind == "NIC":
+        name = device.get(constants.INIC_NAME, None)
+      elif kind == "disk":
+        name = device.get(constants.IDISK_NAME, None)
+      else:
+        raise errors.OpPrereqError("Invalid container kind '%s'" % kind,
+                                   errors.ECODE_INVAL)
+    else:
+      name = device.name
+      # Check that a device name is not the UUID of another device
+      valid.append(device.uuid)
+
+    try:
+      int(name)
+    except (ValueError, TypeError):
+      pass
+    else:
+      raise errors.OpPrereqError("Invalid name '%s'. Purely numeric %s names"
+                                 " are not allowed" % (name, kind),
+                                 errors.ECODE_INVAL)
+
+    if name is not None and name.lower() != constants.VALUE_NONE:
+      if name in valid:
+        raise errors.OpPrereqError("%s name '%s' already used" % (kind, name),
+                                   errors.ECODE_NOTUNIQUE)
+      else:
+        valid.append(name)
diff --git a/test/py/ganeti.utils_unittest.py b/test/py/ganeti.utils_unittest.py
index 343e029..f57b713 100755
--- a/test/py/ganeti.utils_unittest.py
+++ b/test/py/ganeti.utils_unittest.py
@@ -369,5 +369,26 @@ class TestVerifyDictOptions(unittest.TestCase):
                       some_keys, self.defaults)
 
 
+class TestValidateDeviceNames(unittest.TestCase):
+  def testEmpty(self):
+    utils.ValidateDeviceNames("NIC", [])
+    utils.ValidateDeviceNames("disk", [])
+
+  def testNoName(self):
+    nics = [{}, {}]
+    utils.ValidateDeviceNames("NIC", nics)
+
+  def testInvalidName(self):
+    self.assertRaises(errors.OpPrereqError, utils.ValidateDeviceNames,
+                      "disk", [{constants.IDISK_NAME: "42"}])
+    self.assertRaises(errors.OpPrereqError, utils.ValidateDeviceNames,
+                      "NIC", [{constants.INIC_NAME: "42"}])
+
+  def testUsedName(self):
+    disks = [{constants.IDISK_NAME: "name1"}, {constants.IDISK_NAME: "name1"}]
+    self.assertRaises(errors.OpPrereqError, utils.ValidateDeviceNames,
+                      "disk", disks)
+
+
 if __name__ == "__main__":
   testutils.GanetiTestProgram()
-- 
1.7.10.4