Honor disks_active of instance when adding disks

Adding a disk to an instance used to leave the disk behind activated, no
matter how the disks_active flag of the instance was. This changes make
sure that new disks are only active if the other disks of the instance
are active too.

Tests are included.

Signed-off-by: Thomas Thrainer <thomasth@google.com>
Reviewed-by: Jose A. Lopes <jabolopes@google.com>

diff --git a/lib/cmdlib/instance.py b/lib/cmdlib/instance.py
index 93d7ebe..10fccc8 100644 (file)
--- a/lib/cmdlib/instance.py
+++ b/lib/cmdlib/instance.py
@@ -2748,6 +2748,14 @@ class LUInstanceSetParams(LogicalUnit):
                                       constants.DT_EXT),
                                      errors.ECODE_INVAL)
 
+    if not self.op.wait_for_sync and self.instance.disks_active:
+      for mod in self.diskmod:
+        if mod[0] == constants.DDM_ADD:
+          raise errors.OpPrereqError("Can't add a disk to an instance with"
+                                     " activated disks and"
+                                     " --no-wait-for-sync given.",
+                                     errors.ECODE_INVAL)
+
     if self.op.disks and self.instance.disk_template == constants.DT_DISKLESS:
       raise errors.OpPrereqError("Disk operations not supported for"
                                  " diskless instances", errors.ECODE_INVAL)
@@ -3244,6 +3252,11 @@ class LUInstanceSetParams(LogicalUnit):
       raise errors.OpExecError("Failed to sync disks of %s" %
                                self.instance.name)
 
+    # the disk is active at this point, so deactivate it if the instance disks
+    # are supposed to be inactive
+    if not self.instance.disks_active:
+      ShutdownInstanceDisks(self, self.instance, disks=[disk])
+
   @staticmethod
   def _ModifyDisk(idx, disk, params, _):
     """Modifies a disk.

diff --git a/test/py/cmdlib/instance_unittest.py b/test/py/cmdlib/instance_unittest.py
index b1caa62..0ea7191 100644 (file)
--- a/test/py/cmdlib/instance_unittest.py
+++ b/test/py/cmdlib/instance_unittest.py
@@ -1784,6 +1784,10 @@ class TestLUInstanceSetParams(CmdlibTestCase):
       lambda node, _: self.RpcResultsBuilder() \
                         .CreateSuccessfulNodeResult(node, [])
 
+    self.rpc.call_blockdev_shutdown.side_effect = \
+      lambda node, _: self.RpcResultsBuilder() \
+                        .CreateSuccessfulNodeResult(node, [])
+
   def testNoChanges(self):
     op = self.CopyOpCode(self.op)
     self.ExecOpCodeExpectOpPrereqError(op, "No changes submitted")
@@ -2096,7 +2100,18 @@ class TestLUInstanceSetParams(CmdlibTestCase):
     self.ExecOpCodeExpectException(
       op, errors.TypeEnforcementError, "is not a valid size")
 
-  def testAddDisk(self):
+  def testAddDiskRunningInstanceNoWaitForSync(self):
+    op = self.CopyOpCode(self.running_op,
+                         disks=[[constants.DDM_ADD, -1,
+                                 {
+                                   constants.IDISK_SIZE: 1024
+                                 }]],
+                         wait_for_sync=False)
+    self.ExecOpCodeExpectOpPrereqError(
+      op, "Can't add a disk to an instance with activated disks"
+          " and --no-wait-for-sync given.")
+
+  def testAddDiskDownInstance(self):
     op = self.CopyOpCode(self.op,
                          disks=[[constants.DDM_ADD, -1,
                                  {
@@ -2104,6 +2119,18 @@ class TestLUInstanceSetParams(CmdlibTestCase):
                                  }]])
     self.ExecOpCode(op)
 
+    self.assertTrue(self.rpc.call_blockdev_shutdown.called)
+
+  def testAddDiskRunningInstance(self):
+    op = self.CopyOpCode(self.running_op,
+                         disks=[[constants.DDM_ADD, -1,
+                                 {
+                                   constants.IDISK_SIZE: 1024
+                                 }]])
+    self.ExecOpCode(op)
+
+    self.assertFalse(self.rpc.call_blockdev_shutdown.called)
+
   def testAddDiskNoneName(self):
     op = self.CopyOpCode(self.op,
                          disks=[[constants.DDM_ADD, -1,