Merge ~raharper/curtin:feature/format-cmd-flags into curtin:master

Proposed by Ryan Harper
Status: Merged
Approved by: Chad Smith
Approved revision: a53848f08a799706f2058903df8b32ad084ceeef
Merge reported by: Server Team CI bot
Merged at revision: not available
Proposed branch: ~raharper/curtin:feature/format-cmd-flags
Merge into: curtin:master
Diff against target: 147 lines (+45/-2)
6 files modified
curtin/block/mkfs.py (+8/-2)
curtin/block/schemas.py (+1/-0)
doc/topics/storage.rst (+20/-0)
examples/tests/filesystem_battery.yaml (+1/-0)
tests/unittests/test_block_mkfs.py (+8/-0)
tests/vmtests/test_fs_battery.py (+7/-0)
Reviewer Review Type Date Requested Status
Scott Moser (community) Disapprove
Chad Smith Approve
Server Team CI bot continuous-integration Approve
Review via email: mp+381258@code.launchpad.net

Commit message

storage_config: Add 'extra_options' parameter to allow custom mkfs

Allow users to provide additional flags to the mkfs command used to
create a filesystem during storage configuration. The extra_options
value is a string that will be appended to the constructed mkfs
command. The value will be split on whitespace to show up as
individual arguments to the mkfs command.

LP: #1869069

To post a comment you must log in.
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Chad Smith (chad.smith) wrote :

LGTM.

review: Approve
Revision history for this message
Scott Moser (smoser) :
review: Disapprove
Revision history for this message
Scott Moser (smoser) :
Revision history for this message
Ryan Harper (raharper) wrote :

@Scott

That's fair. There's no reason it can't be a list instead. I'll submit another merge with a fix.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/curtin/block/mkfs.py b/curtin/block/mkfs.py
2index 4a1e1f9..570da80 100644
3--- a/curtin/block/mkfs.py
4+++ b/curtin/block/mkfs.py
5@@ -132,7 +132,8 @@ def get_flag_mapping(flag_name, fs_family, param=None, strict=False):
6 return ret
7
8
9-def mkfs(path, fstype, strict=False, label=None, uuid=None, force=False):
10+def mkfs(path, fstype, strict=False, label=None, uuid=None, force=False,
11+ extra_options=None):
12 """Make filesystem on block device with given path using given fstype and
13 appropriate flags for filesystem family.
14
15@@ -146,6 +147,8 @@ def mkfs(path, fstype, strict=False, label=None, uuid=None, force=False):
16
17 Force can be specified to force the mkfs command to continue even if it
18 finds old data or filesystems on the partition.
19+
20+ If extra_options are supplied they are appended to mkfs command.
21 """
22
23 if path is None:
24@@ -201,6 +204,9 @@ def mkfs(path, fstype, strict=False, label=None, uuid=None, force=False):
25 cmd.extend(get_flag_mapping("fatsize", fs_family, param=fat_size,
26 strict=strict))
27
28+ if extra_options:
29+ cmd.extend(extra_options.split())
30+
31 cmd.append(path)
32 util.subp(cmd, capture=True)
33
34@@ -226,6 +232,6 @@ def mkfs_from_config(path, info, strict=False):
35 # NOTE: Since old metadata on partitions that have not been wiped can cause
36 # some mkfs commands to refuse to work, it's best to use force=True
37 mkfs(path, fstype, strict=strict, force=True, uuid=info.get('uuid'),
38- label=info.get('label'))
39+ label=info.get('label'), extra_options=info.get('extra_options'))
40
41 # vi: ts=4 expandtab syntax=python
42diff --git a/curtin/block/schemas.py b/curtin/block/schemas.py
43index a9ed605..981a313 100644
44--- a/curtin/block/schemas.py
45+++ b/curtin/block/schemas.py
46@@ -189,6 +189,7 @@ FORMAT = {
47 'fstype': {'$ref': '#/definitions/fstype'},
48 'label': {'type': 'string'},
49 'volume': {'$ref': '#/definitions/ref_id'},
50+ 'extra_options': {'type': 'string'},
51 }
52 }
53 LVM_PARTITION = {
54diff --git a/doc/topics/storage.rst b/doc/topics/storage.rst
55index f30fc30..16a140a 100644
56--- a/doc/topics/storage.rst
57+++ b/doc/topics/storage.rst
58@@ -472,6 +472,12 @@ curtin will set the uuid of the new filesystem to the specified value.
59
60 If the ``preserve`` key is set to true, curtin will not format the partition.
61
62+**extra_options**: *<string>*
63+
64+The ``extra_options`` key is a string that is appended to the mkfs command used
65+to create the filesystem. **Use of this setting is dangerous. Some flags
66+may cause an error during creation of a filesystem.**
67+
68 **Config Example**::
69
70 - id: disk0-part1-fs1
71@@ -480,6 +486,20 @@ If the ``preserve`` key is set to true, curtin will not format the partition.
72 label: cloud-image
73 volume: disk0-part1
74
75+ - id: disk1-part1-fs1
76+ type: format
77+ fstype: ext4
78+ label: osdata1
79+ uuid: ed51882e-8688-4cd8-97ca-1f2b8bbee458
80+ extra_options: '-O ^metadata_csum,^64bit'
81+
82+ - id: nvme1-part1-fs1
83+ type: format
84+ fstype: ext4
85+ label: cacheset1
86+ extra_options: '-E offset=1024,nodiscard'
87+
88+
89 Mount Command
90 ~~~~~~~~~~~~~
91 The mount command mounts the target filesystem and creates an entry for it in
92diff --git a/examples/tests/filesystem_battery.yaml b/examples/tests/filesystem_battery.yaml
93index 4eae5b6..478efa3 100644
94--- a/examples/tests/filesystem_battery.yaml
95+++ b/examples/tests/filesystem_battery.yaml
96@@ -67,6 +67,7 @@ storage:
97 label: myext4
98 volume: d2p04
99 uuid: 5da136b6-0c41-11e8-a664-525400123456
100+ extra_options: '-O ^ext_attr'
101 - id: fs05
102 type: format
103 fstype: fat16
104diff --git a/tests/unittests/test_block_mkfs.py b/tests/unittests/test_block_mkfs.py
105index 679f85b..b3eec8d 100644
106--- a/tests/unittests/test_block_mkfs.py
107+++ b/tests/unittests/test_block_mkfs.py
108@@ -63,6 +63,14 @@ class TestBlockMkfs(CiTestCase):
109 ["-U", self.test_uuid]]
110 self._run_mkfs_with_config(conf, "mkfs.ext4", expected_flags)
111
112+ def test_mkfs_ext_with_extra_options(self):
113+ conf = self._get_config("ext4")
114+ extra_options = "-D -e continue -E offset=1024,nodiscard,resize=10"
115+ conf['extra_options'] = extra_options
116+ expected_flags = [["-L", "format1"], "-F",
117+ ["-U", self.test_uuid]] + extra_options.split()
118+ self._run_mkfs_with_config(conf, "mkfs.ext4", expected_flags)
119+
120 def test_mkfs_btrfs(self):
121 conf = self._get_config("btrfs")
122 expected_flags = [["--label", "format1"], "--force",
123diff --git a/tests/vmtests/test_fs_battery.py b/tests/vmtests/test_fs_battery.py
124index 6c8cfc2..ecd1729 100644
125--- a/tests/vmtests/test_fs_battery.py
126+++ b/tests/vmtests/test_fs_battery.py
127@@ -101,6 +101,9 @@ class TestFsBattery(VMBaseClass):
128 echo "$part umount: FAIL: $out"
129 done >> battery-mount-umount
130
131+ # collect ext4 features on myext4 partition
132+ dumpe2fs /dev/disk/by-label/myext4 > myext4.dump
133+
134 exit 0
135 """)]
136
137@@ -196,6 +199,10 @@ class TestFsBattery(VMBaseClass):
138 {'/my/bind-over-var-cache/man': 'present',
139 '/my/bind-ro-etc/passwd': 'present'}, paths)
140
141+ def test_ext4_extra_parameters_used_with_mkfs(self):
142+ data = self.load_collect_file("myext4.dump")
143+ self.assertNotIn("ext_attr", data)
144+
145
146 class Centos70XenialTestFsBattery(centos_relbase.centos70_xenial,
147 TestFsBattery):

Subscribers

People subscribed via source and target branches