From 2522b7c40826fdfea19b70999ef00b778049df86 Mon Sep 17 00:00:00 2001 From: Dimitris Aragiorgis Date: Fri, 30 Mar 2012 11:47:14 +0200 Subject: [PATCH] Fix a bug concerning TCP port release Commit f396ad8 returns the TCP port used by DRBD disk back to the TCP/UDP port pool using AddTcpUdpPort(). However, AddTcpUdpPort() writes the config on every invocation, using _WriteConfig(). This causes two problems: * it causes critical errors logged by VerifyConfig(), after the DRBD disk removal, and until the actual instance removal. * if the code following AddTcpUdpPort() fails, the port is already returned back the pool, which causes the port to have duplicates (inconsistent config). AddTcpUdpPort() is invoked in three cases: * during InstanceRemove() through _RemoveDisks(). * during InstanceSetParams() in case of disk removal. * during InstanceSetParams() through _ConvertDrbdToPlain(). This commit fixes the problem by removing the _WriteConfig() call from AddTcpUdpPort(), delegate it to Update() via the TemporaryReservationManager and ensure AddTcpUdpPort() precedes Update(). Signed-off-by: Dimitris Aragiorgis [iustin@google.com: small comments adjustements] Signed-off-by: Iustin Pop Reviewed-by: Iustin Pop (cherry picked from commit 3b3b1bca566a005acd622a5b6e49528e5e3dbe85) --- lib/cmdlib.py | 12 +++++++----- lib/config.py | 7 +++++-- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/lib/cmdlib.py b/lib/cmdlib.py index b287f33..c63ecf0 100644 --- a/lib/cmdlib.py +++ b/lib/cmdlib.py @@ -1,7 +1,7 @@ # # -# Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011 Google Inc. +# Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011, 2012 Google Inc. # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -9665,6 +9665,12 @@ class LUInstanceSetParams(LogicalUnit): child.size = parent.size child.mode = parent.mode + # this is a DRBD disk, return its port to the pool + # NOTE: this must be done right before the call to cfg.Update! + for disk in old_disks: + tcp_port = disk.logical_id[2] + self.cfg.AddTcpUdpPort(tcp_port) + # update instance structure instance.disks = new_disks instance.disk_template = constants.DT_PLAIN @@ -9687,10 +9693,6 @@ class LUInstanceSetParams(LogicalUnit): self.LogWarning("Could not remove metadata for disk %d on node %s," " continuing anyway: %s", idx, pnode, msg) - # this is a DRBD disk, return its port to the pool - for disk in old_disks: - tcp_port = disk.logical_id[2] - self.cfg.AddTcpUdpPort(tcp_port) def Exec(self, feedback_fn): """Modifies an instance. diff --git a/lib/config.py b/lib/config.py index 6d0d860..1ceebd4 100644 --- a/lib/config.py +++ b/lib/config.py @@ -1,7 +1,7 @@ # # -# Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011 Google Inc. +# Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011, 2012 Google Inc. # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -648,12 +648,15 @@ class ConfigWriter: def AddTcpUdpPort(self, port): """Adds a new port to the available port pool. + @warning: this method does not "flush" the configuration (via + L{_WriteConfig}); callers should do that themselves once the + configuration is stable + """ if not isinstance(port, int): raise errors.ProgrammerError("Invalid type passed for port") self._config_data.cluster.tcpudp_port_pool.add(port) - self._WriteConfig() @locking.ssynchronized(_config_lock, shared=1) def GetPortList(self): -- 1.7.10.4