From f5736f3178a69ceeb5eaa9f48aa37d13e41e00c4 Mon Sep 17 00:00:00 2001
From: Andreas Florath <andreas@florath.net>
Date: Thu, 14 Sep 2017 05:14:46 +0000
Subject: [PATCH] block-device lvm: fix umount phase
As described in blockdevice.py detachment and (most) resources
release must be done in the umount phase of a block device module.
Until now these jobs were done in the lvm cleanup() phase - which
is too late - especially when using nested LVMs.
This patch moves the functionality of the cleanup() phase to the
umount() phase for the lvm module.
It includes a test case that fails without applying the provided
source code changes.
Change-Id: I697bfbf042816c5ddf170bde9534cc4f0c7279ff
Signed-off-by: Andreas Florath <andreas@florath.net>
---
diskimage_builder/block_device/level1/lvm.py | 26 +++++++++----------
.../block_device/tests/test_lvm.py | 20 +++++++-------
2 files changed, 24 insertions(+), 22 deletions(-)
diff --git a/diskimage_builder/block_device/level1/lvm.py b/diskimage_builder/block_device/level1/lvm.py
index 1aceb4fd8..2da53d1f7 100644
--- a/diskimage_builder/block_device/level1/lvm.py
+++ b/diskimage_builder/block_device/level1/lvm.py
@@ -155,7 +155,7 @@ class VgsNode(NodeBase):
'devices': self.base,
}
- def _cleanup(self):
+ def _umount(self):
exec_sudo(['vgchange', '-an', self.name])
def get_edges(self):
@@ -214,7 +214,7 @@ class LvsNode(NodeBase):
'device': '/dev/mapper/%s-%s' % (self.base, self.name)
}
- def _cleanup(self):
+ def _umount(self):
exec_sudo(['lvchange', '-an',
'/dev/%s/%s' % (self.base, self.name)])
@@ -280,19 +280,19 @@ class LVMNode(NodeBase):
for lvs in self.lvs:
lvs._create()
- def cleanup(self):
+ def umount(self):
for lvs in self.lvs:
- lvs._cleanup()
+ lvs._umount()
for vgs in self.vgs:
- vgs._cleanup()
+ vgs._umount()
exec_sudo(['udevadm', 'settle'])
-class LVMCleanupNode(NodeBase):
+class LVMUmountNode(NodeBase):
def __init__(self, name, state, pvs):
- """Cleanup Node for LVM
+ """Umount Node for LVM
Information about the PV, VG and LV is typically
cached in lvmetad. Even after removing PV device and
@@ -300,17 +300,17 @@ class LVMCleanupNode(NodeBase):
which leads to a couple of problems.
the 'pvscan --cache' scans the available disks
and updates the cache.
- This must be called after the cleanup of the
+ This must be called after the umount of the
containing block device is done.
"""
- super(LVMCleanupNode, self).__init__(name, state)
+ super(LVMUmountNode, self).__init__(name, state)
self.pvs = pvs
def create(self):
# This class is used for cleanup only
pass
- def cleanup(self):
+ def umount(self):
try:
exec_sudo(['pvscan', '--cache'])
except subprocess.CalledProcessError as cpe:
@@ -412,12 +412,12 @@ class LVMPlugin(PluginBase):
# create the "driver" node
self.lvm_node = LVMNode(config['name'], state,
self.pvs, self.lvs, self.vgs)
- self.lvm_cleanup_node = LVMCleanupNode(
- config['name'] + "-CLEANUP", state, self.pvs)
+ self.lvm_umount_node = LVMUmountNode(
+ config['name'] + "-UMOUNT", state, self.pvs)
def get_nodes(self):
# the nodes for insertion into the graph are all of the pvs,
# vgs and lvs nodes we have created above, the root node and
# the cleanup node.
return self.pvs + self.vgs + self.lvs \
- + [self.lvm_node, self.lvm_cleanup_node]
+ + [self.lvm_node, self.lvm_umount_node]
diff --git a/diskimage_builder/block_device/tests/test_lvm.py b/diskimage_builder/block_device/tests/test_lvm.py
index 14e1d4a9b..3ca37287c 100644
--- a/diskimage_builder/block_device/tests/test_lvm.py
+++ b/diskimage_builder/block_device/tests/test_lvm.py
@@ -21,9 +21,9 @@ from diskimage_builder.block_device.config import config_tree_to_graph
from diskimage_builder.block_device.config import create_graph
from diskimage_builder.block_device.exception import \
BlockDeviceSetupException
-from diskimage_builder.block_device.level1.lvm import LVMCleanupNode
from diskimage_builder.block_device.level1.lvm import LVMNode
from diskimage_builder.block_device.level1.lvm import LVMPlugin
+from diskimage_builder.block_device.level1.lvm import LVMUmountNode
from diskimage_builder.block_device.level1.lvm import LvsNode
from diskimage_builder.block_device.level1.lvm import PvsNode
from diskimage_builder.block_device.level1.lvm import VgsNode
@@ -89,7 +89,7 @@ class TestLVM(tc.TestGraphGeneration):
# XXX: This has not mocked out the "lower" layers of
# creating the devices, which we're assuming works OK, nor
# the upper layers.
- if isinstance(node, (LVMNode, LVMCleanupNode, PvsNode,
+ if isinstance(node, (LVMNode, LVMUmountNode, PvsNode,
VgsNode, LvsNode)):
# only the LVMNode actually does anything here...
node.create()
@@ -198,7 +198,7 @@ class TestLVM(tc.TestGraphGeneration):
# XXX: This has not mocked out the "lower" layers of
# creating the devices, which we're assuming works OK, nor
# the upper layers.
- if isinstance(node, (LVMNode, LVMCleanupNode, PvsNode,
+ if isinstance(node, (LVMNode, LVMUmountNode, PvsNode,
VgsNode, LvsNode)):
# only the PvsNode actually does anything here...
node.create()
@@ -282,7 +282,7 @@ class TestLVM(tc.TestGraphGeneration):
self.assertDictEqual(state['blockdev'], blockdev_state)
#
- # Cleanup test
+ # Umount test
#
with mock.patch(exec_sudo) as mock_exec_sudo, \
mock.patch('tempfile.NamedTemporaryFile') as mock_temp, \
@@ -308,9 +308,9 @@ class TestLVM(tc.TestGraphGeneration):
reverse_order = reversed(call_order)
for node in reverse_order:
- if isinstance(node, (LVMNode, LVMCleanupNode, PvsNode,
+ if isinstance(node, (LVMNode, LVMUmountNode, PvsNode,
VgsNode, LvsNode)):
- node.cleanup()
+ node.umount()
cmd_sequence = [
# delete the lv's
@@ -365,7 +365,7 @@ class TestLVM(tc.TestGraphGeneration):
# XXX: This has not mocked out the "lower" layers of
# creating the devices, which we're assuming works OK, nor
# the upper layers.
- if isinstance(node, (LVMNode, LVMCleanupNode,
+ if isinstance(node, (LVMNode, LVMUmountNode,
PvsNode, VgsNode, LvsNode)):
# only the LVMNode actually does anything here...
node.create()
@@ -409,8 +409,9 @@ class TestLVM(tc.TestGraphGeneration):
reverse_order = reversed(call_order)
for node in reverse_order:
- if isinstance(node, (LVMNode, PvsNode, VgsNode, LvsNode)):
- node.cleanup()
+ if isinstance(node, (LVMNode, LVMUmountNode,
+ PvsNode, VgsNode, LvsNode)):
+ node.umount()
cmd_sequence = [
# deactivate lv's
@@ -422,6 +423,7 @@ class TestLVM(tc.TestGraphGeneration):
mock.call(['vgchange', '-an', 'vg_data']),
mock.call(['udevadm', 'settle']),
+ mock.call(['pvscan', '--cache']),
]
self.assertListEqual(mock_exec_sudo.call_args_list, cmd_sequence)