From 42f25b0b95b023177459f921b55f3c336b434f1b Mon Sep 17 00:00:00 2001 From: Dimitris Aragiorgis <dimara@grnet.gr> Date: Mon, 2 Apr 2012 21:27:35 +0300 Subject: [PATCH] Further fixes concerning drbd port release Commit 3b3b1bc does not entirely fix the bug introduced in commit f396ad8. It fixes consistency of config data in permanent storage, but does not ensure consistency in data held in runtime memory of masterd. The bug of duplicate ports is still triggered when LUInstanceRemove() invokes _RemoveDisks() and this returns False (in case call_blockdev_remove RPC fails). The drbd ports get returned in the pool, but execution is aborted and RemoveInstance() is never invoked. Due to the fact that port handling is not done with TemporaryReservationManager, ensure that ports are released, only if disk related config data is deleted. In _RemoveDisks() release ports only if all RPCs succeed. Extend _RemoveDisks() to include ignore_failures argument passed by _RemoveInstance() to handle the ports appropriately. Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr> Reviewed-by: Michael Hanselmann <hansmi@google.com> --- lib/cmdlib.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/cmdlib.py b/lib/cmdlib.py index c63ecf0ae..5af426c97 100644 --- a/lib/cmdlib.py +++ b/lib/cmdlib.py @@ -5733,7 +5733,7 @@ def _RemoveInstance(lu, feedback_fn, instance, ignore_failures): """ logging.info("Removing block devices for instance %s", instance.name) - if not _RemoveDisks(lu, instance): + if not _RemoveDisks(lu, instance, ignore_failures=ignore_failures): if not ignore_failures: raise errors.OpExecError("Can't remove instance's disks") feedback_fn("Warning: can't remove instance's disks") @@ -6862,7 +6862,7 @@ def _CreateDisks(lu, instance, to_skip=None, target_node=None): _CreateBlockDev(lu, node, instance, device, f_create, info, f_create) -def _RemoveDisks(lu, instance, target_node=None): +def _RemoveDisks(lu, instance, target_node=None, ignore_failures=False): """Remove all disks for an instance. This abstracts away some work from `AddInstance()` and @@ -6883,6 +6883,7 @@ def _RemoveDisks(lu, instance, target_node=None): logging.info("Removing block devices for instance %s", instance.name) all_result = True + ports_to_release = set() for device in instance.disks: if target_node: edata = [(target_node, device)] @@ -6898,8 +6899,11 @@ def _RemoveDisks(lu, instance, target_node=None): # if this is a DRBD disk, return its port to the pool if device.dev_type in constants.LDS_DRBD: - tcp_port = device.logical_id[2] - lu.cfg.AddTcpUdpPort(tcp_port) + ports_to_release.add(device.logical_id[2]) + + if all_result or ignore_failures: + for port in ports_to_release: + lu.cfg.AddTcpUdpPort(port) if instance.disk_template == constants.DT_FILE: file_storage_dir = os.path.dirname(instance.disks[0].logical_id[1]) -- GitLab