I take my objections back, it just took me a while to realize what was
actually happening.
On Thu, Sep 11, 2014 at 03:29:25PM +0200, Petr Pudlak wrote:
As this changes the semantics of the command, this might disrupt how
people are using the command currently. What about the other option,
just updating the documentation, stating that 'modify --online' is
idempontent, would there be any harm in that?
On Thu, Sep 11, 2014 at 01:00:12AM +0300, Dimitris Aragiorgis wrote:
From the man page, the --online option is supposed to mark an
instance down only if it is already offline. Otherwise it should
fail. With this patch we avoid undesired transitions to ADMIN_down
state while the instance is already up and running.
Fix the corresponding QA test as well (TestInstanceModify).
Signed-off-by: Dimitris Aragiorgis <[email protected]>
---
lib/cmdlib/instance.py | 9 +++++++--
qa/qa_instance.py | 5 +++--
2 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/lib/cmdlib/instance.py b/lib/cmdlib/instance.py
index 3e5a2cd..b7a408b 100644
--- a/lib/cmdlib/instance.py
+++ b/lib/cmdlib/instance.py
@@ -2849,9 +2849,14 @@ class LUInstanceSetParams(LogicalUnit):
ispec[constants.ISPEC_DISK_COUNT] = len(disk_sizes)
ispec[constants.ISPEC_DISK_SIZE] = disk_sizes
- if self.op.offline is not None and self.op.offline:
+ # either --online or --offline was passed
+ if self.op.offline is not None:
+ if self.op.offline:
+ msg = "can't change to offline without being down first"
+ else:
+ msg = "can't change to online (down) without being offline first"
CheckInstanceState(self, self.instance, CAN_CHANGE_INSTANCE_OFFLINE,
- msg="can't change to offline")
+ msg=msg)
def CheckPrereq(self):
"""Check prerequisites.
diff --git a/qa/qa_instance.py b/qa/qa_instance.py
index 0e66be1..abd2cc6 100644
--- a/qa/qa_instance.py
+++ b/qa/qa_instance.py
@@ -554,8 +554,9 @@ def TestInstanceModify(instance):
AssertCommand(["gnt-instance", "modify", "--offline", instance.name],
fail=True)
- # ...while making it online is ok, and should work
- AssertCommand(["gnt-instance", "modify", "--online", instance.name])
+ # ...while making it online fails too (needs to be offline first)
+ AssertCommand(["gnt-instance", "modify", "--online", instance.name],
+ fail=True)
@InstanceCheck(INST_UP, INST_UP, FIRST_ARG)
--
1.7.10.4
