sig_cloud/diskimage-builder
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Don't fstrim vfat partitions

Browse Source 
This small change avoids running fstrim on vfat partitions.

The mount order test-case has been updated to also test the mkfs
creation components, and the input config modified to have a vfat
partition to cover this path.

Change-Id: I8952e748d4bdc12a5769706de9057c1e97d95e37
This commit is contained in:
Ian Wienand 2018-01-18 08:31:31 +11:00
parent d940ae6775
commit fd9a8acecd
4 changed files with 70 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
diskimage_builder/block_device/level3/mount.py
View File

@ -95,17 +95,20 @@ class MountPointNode(NodeBase):
def umount(self): def umount(self):
logger.info("Called for [%s]", self.name) logger.info("Called for [%s]", self.name)
# Before calling umount call 'fstrim' on the mounted file # Before calling umount, call 'fstrim' on suitable mounted
# system. This discards unused blocks from the mounted # file systems. This discards unused blocks from the mounted
# file system and therefore decreases the resulting image # file system and therefore decreases the resulting image
# size. # size.
# A race condition can occur when trying to fstrim immediately after #
# deleting a file resulting in that free space not being reclaimed. # A race condition can occur when trying to fstrim immediately
# Calling sync before fstrim is a workaround for this behaviour. # after deleting a file resulting in that free space not being
# reclaimed. Calling sync before fstrim is a workaround for
# this behaviour.
# https://lists.gnu.org/archive/html/qemu-devel/2014-03/msg02978.html # https://lists.gnu.org/archive/html/qemu-devel/2014-03/msg02978.html
exec_sudo(["sync"]) exec_sudo(["sync"])
exec_sudo(["fstrim", "--verbose", if self.state['filesys'][self.base]['fstype'] != 'vfat':
self.state['mount'][self.mount_point]['path']]) exec_sudo(["fstrim", "--verbose",
self.state['mount'][self.mount_point]['path']])
exec_sudo(["umount", self.state['mount'][self.mount_point]['path']]) exec_sudo(["umount", self.state['mount'][self.mount_point]['path']])
def delete(self): def delete(self):

5
diskimage_builder/block_device/tests/config/multiple_partitions_graph.yaml
View File

@ -21,6 +21,7 @@
base: root base: root
name: mkfs_root name: mkfs_root
type: xfs type: xfs
uuid: root-uuid-1234
- mount: - mount:
base: mkfs_root base: mkfs_root
@ -37,6 +38,7 @@
base: var base: var
name: mkfs_var name: mkfs_var
type: xfs type: xfs
uuid: var-uuid-1234
- mount: - mount:
base: mkfs_var base: mkfs_var
@ -52,7 +54,8 @@
- mkfs: - mkfs:
base: var_log base: var_log
name: mkfs_var_log name: mkfs_var_log
type: xfs type: vfat
label: varlog
- mount: - mount:
base: mkfs_var_log base: mkfs_var_log

5
diskimage_builder/block_device/tests/config/multiple_partitions_tree.yaml
View File

@ -11,6 +11,7 @@
size: 55% size: 55%
mkfs: mkfs:
type: xfs type: xfs
uuid: root-uuid-1234
mount: mount:
mount_point: / mount_point: /
fstab: fstab:
@ -20,6 +21,7 @@
size: 40% size: 40%
mkfs: mkfs:
type: xfs type: xfs
uuid: var-uuid-1234
mount: mount:
mount_point: /var mount_point: /var
fstab: fstab:
@ -28,7 +30,8 @@
- name: var_log - name: var_log
size: 5% size: 5%
mkfs: mkfs:
type: xfs type: vfat
label: varlog
mount: mount:
mount_point: /var/log mount_point: /var/log
fstab: fstab:

77
diskimage_builder/block_device/tests/test_mount_order.py
View File

@ -19,6 +19,7 @@ import diskimage_builder.block_device.tests.test_config as tc
from diskimage_builder.block_device.config import config_tree_to_graph 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.config import create_graph
from diskimage_builder.block_device.level2.mkfs import FilesystemNode
from diskimage_builder.block_device.level3.mount import cmp_mount_order from diskimage_builder.block_device.level3.mount import cmp_mount_order
from diskimage_builder.block_device.level3.mount import MountPointNode from diskimage_builder.block_device.level3.mount import MountPointNode
from diskimage_builder.block_device.tests.test_base import TestBase from diskimage_builder.block_device.tests.test_base import TestBase
@ -59,7 +60,10 @@ class TestMountOrder(tc.TestGraphGeneration):
@mock.patch('diskimage_builder.block_device.level3.mount.exec_sudo', @mock.patch('diskimage_builder.block_device.level3.mount.exec_sudo',
side_effect=_exec_sudo_log) side_effect=_exec_sudo_log)
def test_mount_order(self, mock_exec_sudo): @mock.patch('diskimage_builder.block_device.level2.mkfs.exec_sudo',
side_effect=_exec_sudo_log)
def test_mfks_and_mount_order(self, mock_exec_sudo_mkfs,
mock_exec_sudo_mount):
# XXX: better mocking for the os.path.exists calls to avoid # XXX: better mocking for the os.path.exists calls to avoid
# failing if this exists. # failing if this exists.
self.assertFalse(os.path.exists('/fake/')) self.assertFalse(os.path.exists('/fake/'))
@ -71,36 +75,50 @@ class TestMountOrder(tc.TestGraphGeneration):
graph, call_order = create_graph(config, self.fake_default_config, graph, call_order = create_graph(config, self.fake_default_config,
state) state)
# build up some fake state so that we don't have to mock out # Mocked block device state
# all the parent calls that would really make these values, as state['blockdev'] = {}
# we just want to test MountPointNode state['blockdev']['root'] = {'device': '/dev/loopXp1/root'}
state['filesys'] = { state['blockdev']['var'] = {'device': '/dev/loopXp2/var'}
'mkfs_root': {'device': 'fake_root_device'}, state['blockdev']['var_log'] = {'device': '/dev/loopXp3/var_log'}
'mkfs_var': {'device': 'fake_var_device'},
'mkfs_var_log': {'device': 'fake_var_log_device'}
}
for node in call_order: for node in call_order:
if isinstance(node, MountPointNode): if isinstance(node, (FilesystemNode, MountPointNode)):
node.create() node.create()
for node in reversed(call_order): for node in reversed(call_order):
if isinstance(node, MountPointNode): if isinstance(node, (FilesystemNode, MountPointNode)):
node.umount() node.umount()
# ensure that partitions are mounted in order root->var->var/log # ensure that partitions were mounted in order root->var->var/log
self.assertListEqual(state['mount_order'], ['/', '/var', '/var/log']) self.assertListEqual(state['mount_order'], ['/', '/var', '/var/log'])
# fs creation sequence (note we don't care about order of this
# as they're all independent)
cmd_sequence = [
mock.call(['mkfs', '-t', 'xfs', '-L', 'mkfs_root',
'-m', 'uuid=root-uuid-1234',
'-q', '/dev/loopXp1/root']),
mock.call(['mkfs', '-t', 'xfs', '-L', 'mkfs_var',
'-m', 'uuid=var-uuid-1234',
'-q', '/dev/loopXp2/var']),
mock.call(['mkfs', '-t', 'vfat', '-n', 'varlog',
'/dev/loopXp3/var_log'])
]
self.assertEqual(mock_exec_sudo_mkfs.call_count, len(cmd_sequence))
mock_exec_sudo_mkfs.assert_has_calls(cmd_sequence, any_order=True)
# Check mount sequence
cmd_sequence = [ cmd_sequence = [
# mount sequence # mount sequence
mock.call(['mkdir', '-p', '/fake/']), mock.call(['mkdir', '-p', '/fake/']),
mock.call(['mount', 'fake_root_device', '/fake/']), mock.call(['mount', '/dev/loopXp1/root', '/fake/']),
mock.call(['mkdir', '-p', '/fake/var']), mock.call(['mkdir', '-p', '/fake/var']),
mock.call(['mount', 'fake_var_device', '/fake/var']), mock.call(['mount', '/dev/loopXp2/var', '/fake/var']),
mock.call(['mkdir', '-p', '/fake/var/log']), mock.call(['mkdir', '-p', '/fake/var/log']),
mock.call(['mount', 'fake_var_log_device', '/fake/var/log']), mock.call(['mount', '/dev/loopXp3/var_log', '/fake/var/log']),
# umount sequence # umount sequence
mock.call(['sync']), mock.call(['sync']),
mock.call(['fstrim', '--verbose', '/fake/var/log']), # note /fake/var/log is a vfs partition to make sure
# we don't try to fstrim it
mock.call(['umount', '/fake/var/log']), mock.call(['umount', '/fake/var/log']),
mock.call(['sync']), mock.call(['sync']),
mock.call(['fstrim', '--verbose', '/fake/var']), mock.call(['fstrim', '--verbose', '/fake/var']),
@ -109,13 +127,13 @@ class TestMountOrder(tc.TestGraphGeneration):
mock.call(['fstrim', '--verbose', '/fake/']), mock.call(['fstrim', '--verbose', '/fake/']),
mock.call(['umount', '/fake/']) mock.call(['umount', '/fake/'])
] ]
self.assertListEqual(mock_exec_sudo.call_args_list, cmd_sequence) self.assertListEqual(mock_exec_sudo_mount.call_args_list, cmd_sequence)
@mock.patch('diskimage_builder.block_device.level3.mount.exec_sudo', @mock.patch('diskimage_builder.block_device.level3.mount.exec_sudo',
side_effect=_exec_sudo_log) side_effect=_exec_sudo_log)
def test_mount_order_unsorted(self, mock_exec_sudo): def test_mount_order_unsorted(self, mock_exec_sudo):
# As above, but this is out of order and gets sorted # As above, but this is out of order and gets sorted
# so that root is mounted first. # so that root is mounted first (and skips the mkfs testing).
config = self.load_config_file('lvm_tree_partition_ordering.yaml') config = self.load_config_file('lvm_tree_partition_ordering.yaml')
parsed_graph = config_tree_to_graph(config) parsed_graph = config_tree_to_graph(config)
state = {} state = {}
@ -124,9 +142,18 @@ class TestMountOrder(tc.TestGraphGeneration):
self.fake_default_config, self.fake_default_config,
state) state)
state['filesys'] = { state['filesys'] = {
'mkfs_root': {'device': 'fake_root_device'}, 'mkfs_root': {
'mkfs_var': {'device': 'fake_var_device'}, 'device': '/dev/loopXp1',
'mkfs_boot': {'device': 'fake_boot_device'} 'fstype': 'xfs'
},
'mkfs_var': {
'device': '/dev/loopXp2',
'fstype': 'xfs',
},
'mkfs_boot': {
'device': '/dev/loopXp3',
'fstype': 'vfat',
},
} }
for node in call_order: for node in call_order:
@ -142,17 +169,17 @@ class TestMountOrder(tc.TestGraphGeneration):
cmd_sequence = [ cmd_sequence = [
# mount sequence # mount sequence
mock.call(['mkdir', '-p', '/fake/']), mock.call(['mkdir', '-p', '/fake/']),
mock.call(['mount', 'fake_root_device', '/fake/']), mock.call(['mount', '/dev/loopXp1', '/fake/']),
mock.call(['mkdir', '-p', '/fake/boot']), mock.call(['mkdir', '-p', '/fake/boot']),
mock.call(['mount', 'fake_boot_device', '/fake/boot']), mock.call(['mount', '/dev/loopXp3', '/fake/boot']),
mock.call(['mkdir', '-p', '/fake/var']), mock.call(['mkdir', '-p', '/fake/var']),
mock.call(['mount', 'fake_var_device', '/fake/var']), mock.call(['mount', '/dev/loopXp2', '/fake/var']),
# umount sequence # umount sequence
mock.call(['sync']), mock.call(['sync']),
mock.call(['fstrim', '--verbose', '/fake/var']), mock.call(['fstrim', '--verbose', '/fake/var']),
mock.call(['umount', '/fake/var']), mock.call(['umount', '/fake/var']),
mock.call(['sync']), mock.call(['sync']),
mock.call(['fstrim', '--verbose', '/fake/boot']), # no trim on vfat /fake/boot
mock.call(['umount', '/fake/boot']), mock.call(['umount', '/fake/boot']),
mock.call(['sync']), mock.call(['sync']),
mock.call(['fstrim', '--verbose', '/fake/']), mock.call(['fstrim', '--verbose', '/fake/']),