On Thu, Sep 27, 2012 at 12:20 PM, Michael Hanselmann <han...@google.com> wrote: > 2012/9/27 Bernardo Dal Seno <bdals...@google.com>: >> --- a/lib/cmdlib.py >> +++ b/lib/cmdlib.py >> @@ -7337,6 +7337,9 @@ class LUInstanceRecreateDisks(LogicalUnit): >> if self.op.nodes: >> self.cfg.Update(instance, feedback_fn) >> >> + # All touched nodes must be locked >> + assert all(x in self.owned_locks(locking.LEVEL_NODE) > > Please use compat.all for now. I guess this fix should be backported > to 2.6, which still needs to run on older Python versions. Also you > call owned_locks once for every node; please use a local variable for > that. Actually:
Ack. > assert frozenset(instance.all_nodes) == > frozenset(self.owned_locks(LEVEL_NODE)) I thought about this, but I discarded because it's too strict. The check is to make sure that the nodes that need to be locked are locked, not to make sure that the node that needn't to be locked aren't. This second check, although useful, belongs somewhere else, but it would be very close to the code that does the locking, so I'm not sure it's worth adding. Also, I decided against using issuperset because the body of the for loop is execute at most twice (for DRBD node), so I thought it wan't worth it. What do you think? Bernardo
