Reformulated commit message:

Allow instance disks to be added with --no-wait-for-sync

The patch 3c260845147c6dad35e37c03ba9a7556814f3f3f fixed a bug where
adding a new disk to an instance with deactivated disks activated the
disk. However, it also introduced some erroneous behaviour, preventing
disks from being added to online instances with --no-wait-for-sync.

In line with the original meaning of the patch, this patch modifies the
check to disallow adding disks to shutdown instances with
--no-wait-for-sync, and allow doing so for online instances.

On Monday, August 11, 2014 3:58:24 PM UTC+2, Hrvoje Ribicic wrote:
>
> The patch 3c260845147c6dad35e37c03ba9a7556814f3f3f fixed a bug where 
> adding a new disk to an instance with deactivated disks activated the 
> disk. However, it also introduced some erroneous behaviour, preventing 
> disks from being added to online instances with --no-wait-for-sync. 
>
> In line with the original meaning of the patch, this patch disallows 
> adding disks to shutdown instances with --no-wait-for-sync, as the 
> disks should not be deactivated while syncing. 
>
> Signed-off-by: Hrvoje Ribicic <[email protected]> 
> --- 
>  lib/cmdlib/instance.py              |  4 ++-- 
>  test/py/cmdlib/instance_unittest.py | 16 +++++++++++++--- 
>  2 files changed, 15 insertions(+), 5 deletions(-) 
>
> diff --git a/lib/cmdlib/instance.py b/lib/cmdlib/instance.py 
> index 480198d..3e5a2cd 100644 
> --- a/lib/cmdlib/instance.py 
> +++ b/lib/cmdlib/instance.py 
> @@ -2819,11 +2819,11 @@ class LUInstanceSetParams(LogicalUnit): 
>                                        constants.DT_EXT), 
>                                       errors.ECODE_INVAL) 
>   
> -    if not self.op.wait_for_sync and self.instance.disks_active: 
> +    if not self.op.wait_for_sync and not 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" 
> +                                     " deactivated disks and" 
>                                       " --no-wait-for-sync given.", 
>                                       errors.ECODE_INVAL) 
>   
> diff --git a/test/py/cmdlib/instance_unittest.py 
> b/test/py/cmdlib/instance_unittest.py 
> index 245f023..2ecb816 100644 
> --- a/test/py/cmdlib/instance_unittest.py 
> +++ b/test/py/cmdlib/instance_unittest.py 
> @@ -2159,9 +2159,8 @@ class TestLUInstanceSetParams(CmdlibTestCase): 
>                                     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.") 
> +    self.ExecOpCode(op) 
> +    self.assertFalse(self.rpc.call_blockdev_shutdown.called) 
>   
>    def testAddDiskDownInstance(self): 
>      op = self.CopyOpCode(self.op, 
> @@ -2173,6 +2172,17 @@ class TestLUInstanceSetParams(CmdlibTestCase): 
>   
>      self.assertTrue(self.rpc.call_blockdev_shutdown.called) 
>   
> +  def testAddDiskDownInstanceNoWaitForSync(self): 
> +    op = self.CopyOpCode(self.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 deactivated disks" 
> +          " and --no-wait-for-sync given.") 
> + 
>    def testAddDiskRunningInstance(self): 
>      op = self.CopyOpCode(self.running_op, 
>                           disks=[[constants.DDM_ADD, -1, 
> -- 
> 2.0.0.526.g5318336 
>
>

Reply via email to