Assign unique filenames to filebased disks

With the new format for cmdline arguments, the user is able to add a
disk to an instance at a specific index. But filebased disks' filenames
have the form "{0}/disk{1}" where '{0}' is the file_storage_dir and
'{1}' is the index of the disk. So if an instance has 3 disks and we
try to create a new one at index 1, the operation will fail because the
filename "{0}/disk1" already exists.

This patch fixes the above problem and also makes the naming of file and
shared disks uniform with other templates.

Signed-off-by: Ilias Tsitsimpis <iliastsi@grnet.gr>
Signed-off-by: Klaus Aehlig <aehlig@google.com>
Reviewed-by: Klaus Aehlig <aehlig@google.com>

diff --git a/lib/cmdlib/instance_storage.py b/lib/cmdlib/instance_storage.py
index 28c2c9d..333ae09 100644 (file)
--- a/lib/cmdlib/instance_storage.py
+++ b/lib/cmdlib/instance_storage.py
@@ -52,6 +52,8 @@ _DISK_TEMPLATE_NAME_PREFIX = {
   constants.DT_PLAIN: "",
   constants.DT_RBD: ".rbd",
   constants.DT_EXT: ".ext",
+  constants.DT_FILE: ".file",
+  constants.DT_SHARED_FILE: ".sharedfile",
   }
 
 
@@ -464,8 +466,8 @@ def GenerateDiskTemplate(
     elif template_name in (constants.DT_FILE, constants.DT_SHARED_FILE):
       logical_id_fn = \
         lambda _, disk_index, disk: (file_driver,
-                                     "%s/disk%d" % (file_storage_dir,
-                                                    disk_index))
+                                     "%s/%s" % (file_storage_dir,
+                                                names[idx]))
     elif template_name == constants.DT_BLOCK:
       logical_id_fn = \
         lambda idx, disk_index, disk: (constants.BLOCKDEV_DRIVER_MANUAL,

diff --git a/test/py/ganeti.cmdlib_unittest.py b/test/py/ganeti.cmdlib_unittest.py
index 1165eef..55dea3c 100755 (executable)
--- a/test/py/ganeti.cmdlib_unittest.py
+++ b/test/py/ganeti.cmdlib_unittest.py
@@ -23,6 +23,7 @@
 
 
 import os
+import re
 import unittest
 import tempfile
 import shutil
@@ -1325,11 +1326,12 @@ class TestGenerateDiskTemplate(unittest.TestCase):
         disk_template, file_storage_dir="/tmp",
         file_driver=constants.FD_BLKTAP)
 
-      self.assertEqual(map(operator.attrgetter("logical_id"), result), [
-        (constants.FD_BLKTAP, "/tmp/disk2"),
-        (constants.FD_BLKTAP, "/tmp/disk3"),
-        (constants.FD_BLKTAP, "/tmp/disk4"),
-        ])
+      for (idx, disk) in enumerate(result):
+        (file_driver, file_storage_dir) = disk.logical_id
+        dir_fmt = r"^/tmp/.*\.%s\.disk%d$" % (disk_template, idx + 2)
+        self.assertEqual(file_driver, constants.FD_BLKTAP)
+        # FIXME: use assertIsNotNone when py 2.7 is minimum supported version
+        self.assertNotEqual(re.match(dir_fmt, file_storage_dir), None)
 
   def testBlock(self):
     disk_info = [{