Merge lp:~raharper/curtin/trunk.nvme_bcache into lp:~curtin-dev/curtin/trunk

Proposed by Ryan Harper
Status: Merged
Merged at revision: 492
Proposed branch: lp:~raharper/curtin/trunk.nvme_bcache
Merge into: lp:~curtin-dev/curtin/trunk
Diff against target: 684 lines (+559/-22)
6 files modified
curtin/block/clear_holders.py (+81/-14)
curtin/util.py (+21/-0)
examples/tests/nvme_bcache.yaml (+114/-0)
tests/unittests/test_clear_holders.py (+197/-8)
tests/unittests/test_util.py (+69/-0)
tests/vmtests/test_nvme.py (+77/-0)
To merge this branch: bzr merge lp:~raharper/curtin/trunk.nvme_bcache
Reviewer Review Type Date Requested Status
Server Team CI bot continuous-integration Approve
Scott Moser (community) Approve
Review via email: mp+322307@code.launchpad.net

Description of the change

clear-holders: ensure that bcache handler waits bcache devices to shutdown

Clear-holder's bcache shutdown handler does not wait for devices owned by bcache driver to be released before returning. On sufficiently fast systems (and with multiple cpus) the time for bcache to release the backing devices may occur slower than the time it takes for curtin to
start attempting to wipe partition data on a backing device. This results
in an IO error due to curtin asserting exclusive access to a device for
wiping.

This series introduces some additional logic in the bcache shutdown
handler by testing that the bcache devices are removed before returning
from the handler.

If the underlying devices are not released, the handler raises an
exception.

Fixes: LP:#1680409

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
Scott Moser (smoser) wrote :

Some comments inline.
The unit tests seem really tied to the implementation (mocking os.path.exists which is called by 2 different callers).

I realize this is tricky, but maybe some re-work could make the tests more clear.

A test for get_bcache_sys_path would be good.

Revision history for this message
Ryan Harper (raharper) wrote :
Download full text (7.5 KiB)

On Tue, Apr 11, 2017 at 9:17 AM, Scott Moser <email address hidden> wrote:

> Some comments inline.
> The unit tests seem really tied to the implementation (mocking
> os.path.exists which is called by 2 different callers).
>
> I realize this is tricky, but maybe some re-work could make the tests more
> clear.
>

I'm open to reworking them. Not 100% happy with the current coverage.

>
>
> A test for get_bcache_sys_path would be good.
>

Pretty sure I have that.

>
>
> Diff comments:
>
> > === modified file 'curtin/block/clear_holders.py'
> > --- curtin/block/clear_holders.py 2017-01-31 22:03:02 +0000
> > +++ curtin/block/clear_holders.py 2017-04-10 18:04:42 +0000
> > @@ -56,32 +56,75 @@
> >
> > def get_bcache_using_dev(device):
> > """
> > - Get the /sys/fs/bcache/ path of the bcache volume using specified
> device
> > + Get the /sys/fs/bcache/ path of the bcache cache device bound to
> > + specified device
> > """
> > # FIXME: when block.bcache is written this should be moved there
> > sysfs_path = block.sys_block_path(device)
> > return os.path.realpath(os.path.join(sysfs_path, 'bcache',
> 'cache'))
> >
> >
> > +def get_bcache_sys_path(device):
> > + """
> > + Get the /sys/class/block/<device>/bcache path if present
> > + """
> > + sysfs_path = block.sys_block_path(device)
> > + return os.path.realpath(os.path.join(sysfs_path, 'bcache'))
>
> The comment says "if present", but its not clear what the return would be.
> If the *device* does not exist (get_bcache_sys_path("bogusdev1")) then
> you'll
> raise an OSError, but if the device exists and the 'bcache' entry does not
> you'll just return it either way.
>

> >>> os.path.realpath("/bogus/path/here")
> '/bogus/path/here'
>

Ack, the comment was really from the caller;
I'll rework as I'd like it validate the path before the realpath call as
well.

>
> > +
> > +
> > def shutdown_bcache(device):
> > """
> > Shut down bcache for specified bcache device
> > +
> > + 1. Stop the cacheset that `device` is connected to
> > + 2. Stop the 'device'
> > """
> > + if not device.startswith('/sys/class/block'):
>
> we use the string '/sys/class/block' other places, we really should
> SYS_CLASS_BLOCK = '/sys/block'
> not "your fault", but i think having that constant would be good.
>

ACK, though I'd like to punt on the general cleanup into a separate MP.

>
> > + raise ValueError('Invalid Device (%s): '
> > + 'Device path must start with
> /sys/class/block/',
> > + device)
> > +
> > bcache_shutdown_message = ('shutdown_bcache running on {} has
> determined '
> > 'that the device has already been shut
> down '
> > 'during handling of another bcache dev. '
> > 'skipping'.format(device))
> > - if not os.path.exists(device):
> > - LOG.info(bcache_shutdown_message)
> > - return
> > -
> > - bcache_sysfs = get_bcache_using_dev(device)
> > - if not os.path.exists(bcache_sysfs):
> > - LOG.info(bcache_shutdown_message)
> > - return
> > -
> ...

Read more...

493. By Ryan Harper

Add strict mode for bcache sysfs paths and longer, frequent retries

Feedback from MP suggested we should apply some path checking
when handling sysfs bcache entries; add the strict mode used in
other block methods.

When waiting for file removal, we want to avoid blocking for long
periods and also ensure we've waited long enough. A bcache device
may need to flush a cache to the backing device which could take
some time. We also don't want to add latency if it isn't needed by
incurring long sleep intervals. Now we will wait 30 seconds in total
and check every 0.2 seconds if the device has been removed.

Revision history for this message
Scott Moser (smoser) :
review: Approve
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
494. By Ryan Harper

merge from trunk

Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
495. By Ryan Harper

merge from trunk

Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'curtin/block/clear_holders.py'
--- curtin/block/clear_holders.py 2017-01-31 22:03:02 +0000
+++ curtin/block/clear_holders.py 2017-04-24 17:53:29 +0000
@@ -21,6 +21,7 @@
21having to reboot the system21having to reboot the system
22"""22"""
2323
24import errno
24import os25import os
2526
26from curtin import (block, udev, util)27from curtin import (block, udev, util)
@@ -54,34 +55,100 @@
54 return out.strip()55 return out.strip()
5556
5657
57def get_bcache_using_dev(device):58def get_bcache_using_dev(device, strict=True):
58 """59 """
59 Get the /sys/fs/bcache/ path of the bcache volume using specified device60 Get the /sys/fs/bcache/ path of the bcache cache device bound to
61 specified device
60 """62 """
61 # FIXME: when block.bcache is written this should be moved there63 # FIXME: when block.bcache is written this should be moved there
62 sysfs_path = block.sys_block_path(device)64 sysfs_path = block.sys_block_path(device)
63 return os.path.realpath(os.path.join(sysfs_path, 'bcache', 'cache'))65 path = os.path.realpath(os.path.join(sysfs_path, 'bcache', 'cache'))
66 if strict and not os.path.exists(path):
67 err = OSError(
68 "device '{}' did not have existing syspath '{}'".format(
69 device, path))
70 err.errno = errno.ENOENT
71 raise err
72
73 return path
74
75
76def get_bcache_sys_path(device, strict=True):
77 """
78 Get the /sys/class/block/<device>/bcache path
79 """
80 sysfs_path = block.sys_block_path(device)
81 path = os.path.join(sysfs_path, 'bcache')
82 if strict and not os.path.exists(path):
83 err = OSError(
84 "device '{}' did not have existing syspath '{}'".format(
85 device, path))
86 err.errno = errno.ENOENT
87 raise err
88
89 return path
6490
6591
66def shutdown_bcache(device):92def shutdown_bcache(device):
67 """93 """
68 Shut down bcache for specified bcache device94 Shut down bcache for specified bcache device
95
96 1. Stop the cacheset that `device` is connected to
97 2. Stop the 'device'
69 """98 """
99 if not device.startswith('/sys/class/block'):
100 raise ValueError('Invalid Device (%s): '
101 'Device path must start with /sys/class/block/',
102 device)
103
104 # bcache device removal should be fast but in an extreme
105 # case, might require the cache device to flush large
106 # amounts of data to a backing device. The strategy here
107 # is to wait for approximately 30 seconds but to check
108 # frequently since curtin cannot proceed until devices
109 # cleared.
110 removal_retries = [0.2] * 150 # 30 seconds total
70 bcache_shutdown_message = ('shutdown_bcache running on {} has determined '111 bcache_shutdown_message = ('shutdown_bcache running on {} has determined '
71 'that the device has already been shut down '112 'that the device has already been shut down '
72 'during handling of another bcache dev. '113 'during handling of another bcache dev. '
73 'skipping'.format(device))114 'skipping'.format(device))
74 if not os.path.exists(device):115
75 LOG.info(bcache_shutdown_message)116 if not os.path.exists(device):
76 return117 LOG.info(bcache_shutdown_message)
77118 return
78 bcache_sysfs = get_bcache_using_dev(device)119
79 if not os.path.exists(bcache_sysfs):120 # stop cacheset if it exists
80 LOG.info(bcache_shutdown_message)121 bcache_cache_sysfs = get_bcache_using_dev(device)
81 return122 if not os.path.exists(bcache_cache_sysfs):
82123 LOG.info('bcache cacheset already removed: %s',
83 LOG.debug('stopping bcache at: %s', bcache_sysfs)124 os.path.basename(bcache_cache_sysfs))
84 util.write_file(os.path.join(bcache_sysfs, 'stop'), '1', mode=None)125 else:
126 LOG.info('stopping bcache cacheset at: %s', bcache_cache_sysfs)
127 util.write_file(os.path.join(bcache_cache_sysfs, 'stop'),
128 '1', mode=None)
129 try:
130 util.wait_for_removal(bcache_cache_sysfs, retries=removal_retries)
131 except OSError:
132 LOG.info('Failed to stop bcache cacheset %s', bcache_cache_sysfs)
133 raise
134
135 # after stopping cache set, we may need to stop the device
136 if not os.path.exists(device):
137 LOG.info('bcache backing device already removed: %s', device)
138 return
139 else:
140 bcache_block_sysfs = get_bcache_sys_path(device)
141 LOG.info('stopping bcache backing device at: %s', bcache_block_sysfs)
142 util.write_file(os.path.join(bcache_block_sysfs, 'stop'),
143 '1', mode=None)
144 try:
145 util.wait_for_removal(device, retries=removal_retries)
146 except OSError:
147 LOG.info('Failed to stop bcache backing device %s',
148 bcache_block_sysfs)
149 raise
150
151 return
85152
86153
87def shutdown_lvm(device):154def shutdown_lvm(device):
88155
=== modified file 'curtin/util.py'
--- curtin/util.py 2017-04-12 01:38:04 +0000
+++ curtin/util.py 2017-04-24 17:53:29 +0000
@@ -194,6 +194,27 @@
194 return _subp(*args, **kwargs)194 return _subp(*args, **kwargs)
195195
196196
197def wait_for_removal(path, retries=[1, 3, 5, 7]):
198 if not path:
199 raise ValueError('wait_for_removal: missing path parameter')
200
201 # Retry with waits between checking for existence
202 LOG.debug('waiting for %s to be removed', path)
203 for num, wait in enumerate(retries):
204 if not os.path.exists(path):
205 LOG.debug('%s has been removed', path)
206 return
207 LOG.debug('sleeping %s', wait)
208 time.sleep(wait)
209
210 # final check
211 if not os.path.exists(path):
212 LOG.debug('%s has been removed', path)
213 return
214
215 raise OSError('Timeout exceeded for removal of %s', path)
216
217
197def load_command_environment(env=os.environ, strict=False):218def load_command_environment(env=os.environ, strict=False):
198219
199 mapping = {'scratch': 'WORKING_DIR', 'fstab': 'OUTPUT_FSTAB',220 mapping = {'scratch': 'WORKING_DIR', 'fstab': 'OUTPUT_FSTAB',
200221
=== added file 'examples/tests/nvme_bcache.yaml'
--- examples/tests/nvme_bcache.yaml 1970-01-01 00:00:00 +0000
+++ examples/tests/nvme_bcache.yaml 2017-04-24 17:53:29 +0000
@@ -0,0 +1,114 @@
1showtrace: true
2storage:
3 config:
4 - grub_device: true
5 id: sda
6 model: LOGICAL VOLUME
7 name: sda
8 ptable: gpt
9 path: /dev/vdb
10 type: disk
11 wipe: superblock
12 - id: sdb
13 model: LOGICAL VOLUME
14 name: sdb
15 path: /dev/vdc
16 type: disk
17 wipe: superblock
18 - id: nvme0n1
19 model: INTEL SSDPEDME400G4
20 name: nvme0n1
21 ptable: gpt
22 path: /dev/nvme0n1
23 type: disk
24 wipe: superblock
25 - device: sda
26 flag: boot
27 id: sda-part1
28 name: sda-part1
29 number: 1
30 offset: 4194304B
31 size: 536870912B
32 type: partition
33 uuid: 9f0fda16-c096-4cee-82ac-4f5f294253a2
34 wipe: superblock
35 - device: sda
36 flag: boot
37 id: sda-part2
38 name: sda-part2
39 number: 2
40 size: 1G
41 type: partition
42 uuid: 7dbb079c-60e5-4900-b3ca-4a92c5cbd6d3
43 wipe: superblock
44 - device: sda
45 id: sda-part3
46 name: sda-part3
47 number: 3
48 size: 5G
49 type: partition
50 uuid: 560de16e-6dc1-4e85-8e66-aaf1062dabcd
51 wipe: superblock
52 - device: nvme0n1
53 id: nvme0n1-part1
54 name: nvme0n1-part1
55 number: 1
56 offset: 4194304B
57 size: 5G
58 type: partition
59 uuid: 9692359a-8a6d-4fa2-bbe6-48b97c4c8832
60 wipe: superblock
61 - backing_device: sda-part3
62 cache_device: nvme0n1-part1
63 cache_mode: writeback
64 id: bcache1
65 name: bcache1
66 type: bcache
67 - backing_device: sdb
68 cache_device: nvme0n1-part1
69 cache_mode: writeback
70 id: bcache2
71 name: bcache2
72 type: bcache
73 - fstype: fat32
74 id: sda-part1_format
75 label: efi
76 type: format
77 uuid: e23b3115-228d-410c-a2b0-5bffeaf1c1d0
78 volume: sda-part1
79 - fstype: ext4
80 id: sda-part2_format
81 label: boot
82 type: format
83 uuid: 8940edad-7882-4978-91a6-5f80f95ea4e8
84 volume: sda-part2
85 - fstype: ext4
86 id: bcache1_format
87 label: root
88 type: format
89 uuid: 627296af-d976-4baf-8abc-b388143d4dd3
90 volume: bcache1
91 - fstype: xfs
92 id: bcache2_format
93 label: ''
94 type: format
95 uuid: ada2cfb1-cfad-4d66-99ff-ea3e79790233
96 volume: bcache2
97 - device: bcache1_format
98 id: bcache1_mount
99 path: /
100 type: mount
101 - device: sda-part2_format
102 id: sda-part2_mount
103 path: /boot
104 type: mount
105 - device: sda-part1_format
106 id: sda-part1_mount
107 path: /boot/efi
108 type: mount
109 - device: bcache2_format
110 id: bcache2_mount
111 options: ''
112 path: /srv
113 type: mount
114 version: 1
0115
=== modified file 'tests/unittests/test_clear_holders.py'
--- tests/unittests/test_clear_holders.py 2017-01-27 01:00:39 +0000
+++ tests/unittests/test_clear_holders.py 2017-04-24 17:53:29 +0000
@@ -9,6 +9,7 @@
9class TestClearHolders(TestCase):9class TestClearHolders(TestCase):
10 test_blockdev = '/dev/null'10 test_blockdev = '/dev/null'
11 test_syspath = '/sys/class/block/null'11 test_syspath = '/sys/class/block/null'
12 remove_retries = [0.2] * 150 # clear_holders defaults to 30 seconds
12 example_holders_trees = [13 example_holders_trees = [
13 [{'device': '/sys/class/block/sda', 'name': 'sda', 'holders':14 [{'device': '/sys/class/block/sda', 'name': 'sda', 'holders':
14 [{'device': '/sys/class/block/sda/sda1', 'name': 'sda1',15 [{'device': '/sys/class/block/sda/sda1', 'name': 'sda1',
@@ -89,12 +90,25 @@
89 def test_get_bcache_using_dev(self, mock_os, mock_block):90 def test_get_bcache_using_dev(self, mock_os, mock_block):
90 """Ensure that get_bcache_using_dev works"""91 """Ensure that get_bcache_using_dev works"""
91 fake_bcache = '/sys/fs/bcache/fake'92 fake_bcache = '/sys/fs/bcache/fake'
93 mock_os.path.join.side_effect = os.path.join
94 mock_block.sys_block_path.return_value = self.test_syspath
92 mock_os.path.realpath.return_value = fake_bcache95 mock_os.path.realpath.return_value = fake_bcache
93 mock_os.path.exists.side_effect = lambda x: x == fake_bcache96
94 mock_block.sys_block_path.return_value = self.test_blockdev
95 bcache_dir = clear_holders.get_bcache_using_dev(self.test_blockdev)97 bcache_dir = clear_holders.get_bcache_using_dev(self.test_blockdev)
98 mock_os.path.realpath.assert_called_with(self.test_syspath +
99 '/bcache/cache')
96 self.assertEqual(bcache_dir, fake_bcache)100 self.assertEqual(bcache_dir, fake_bcache)
97101
102 @mock.patch('curtin.block.clear_holders.os')
103 @mock.patch('curtin.block.clear_holders.block')
104 def test_get_bcache_sys_path(self, mock_block, mock_os):
105 fake_backing = '/sys/class/block/fake'
106 mock_block.sys_block_path.return_value = fake_backing
107 mock_os.path.join.side_effect = os.path.join
108 mock_os.path.exists.return_value = True
109 bcache_dir = clear_holders.get_bcache_sys_path("/dev/fake")
110 self.assertEqual(bcache_dir, fake_backing + "/bcache")
111
98 @mock.patch('curtin.block.clear_holders.get_dmsetup_uuid')112 @mock.patch('curtin.block.clear_holders.get_dmsetup_uuid')
99 @mock.patch('curtin.block.clear_holders.block')113 @mock.patch('curtin.block.clear_holders.block')
100 def test_differentiate_lvm_and_crypt(114 def test_differentiate_lvm_and_crypt(
@@ -115,22 +129,197 @@
115 mock_block.path_to_kname.assert_called_with(self.test_syspath)129 mock_block.path_to_kname.assert_called_with(self.test_syspath)
116 mock_get_dmsetup_uuid.assert_called_with(self.test_syspath)130 mock_get_dmsetup_uuid.assert_called_with(self.test_syspath)
117131
132 @mock.patch('curtin.block.clear_holders.get_bcache_sys_path')
118 @mock.patch('curtin.block.clear_holders.util')133 @mock.patch('curtin.block.clear_holders.util')
119 @mock.patch('curtin.block.clear_holders.os')134 @mock.patch('curtin.block.clear_holders.os')
120 @mock.patch('curtin.block.clear_holders.LOG')135 @mock.patch('curtin.block.clear_holders.LOG')
121 @mock.patch('curtin.block.clear_holders.get_bcache_using_dev')136 @mock.patch('curtin.block.clear_holders.get_bcache_using_dev')
122 def test_shutdown_bcache(self, mock_get_bcache, mock_log, mock_os,137 def test_shutdown_bcache(self, mock_get_bcache, mock_log, mock_os,
123 mock_util):138 mock_util, mock_get_bcache_block):
124 """test clear_holders.shutdown_bcache"""139 """test clear_holders.shutdown_bcache"""
140 #
141 # pass in a sysfs path to a bcache block device,
142 # determine the bcache cset it is part of (or not)
143 # 1) stop the cset device (if it's enabled)
144 # 2) wait on cset to be removed if it was present
145 # 3) stop the block device (if it's still present after stopping cset)
146 # 4) wait on bcache block device to be removed
147 #
148
149 device = self.test_syspath
150 bcache_cset_uuid = 'c08ae789-a964-46fb-a66e-650f0ae78f94'
151
125 mock_os.path.exists.return_value = True152 mock_os.path.exists.return_value = True
126 mock_os.path.join.side_effect = os.path.join153 mock_os.path.join.side_effect = os.path.join
127 mock_get_bcache.return_value = self.test_blockdev154 # os.path.realpath on symlink of /sys/class/block/null/bcache/cache ->
128 clear_holders.shutdown_bcache(self.test_syspath)155 # to /sys/fs/bcache/cset_UUID
129 mock_get_bcache.assert_called_with(self.test_syspath)156 mock_get_bcache.return_value = '/sys/fs/bcache/' + bcache_cset_uuid
130 self.assertTrue(mock_log.debug.called)157 mock_get_bcache_block.return_value = device + '/bcache'
158
159 clear_holders.shutdown_bcache(device)
160
161 mock_get_bcache.assert_called_with(device)
162 mock_get_bcache_block.assert_called_with(device)
163
164 self.assertTrue(mock_log.info.called)
131 self.assertFalse(mock_log.warn.called)165 self.assertFalse(mock_log.warn.called)
132 mock_util.write_file.assert_called_with(self.test_blockdev + '/stop',166 mock_util.wait_for_removal.assert_has_calls([
167 mock.call('/sys/fs/bcache/' + bcache_cset_uuid,
168 retries=self.remove_retries),
169 mock.call(device, retries=self.remove_retries)])
170
171 mock_util.write_file.assert_has_calls([
172 mock.call('/sys/fs/bcache/%s/stop' % bcache_cset_uuid,
173 '1', mode=None),
174 mock.call(device + '/bcache/stop',
175 '1', mode=None)])
176
177 @mock.patch('curtin.block.clear_holders.get_bcache_sys_path')
178 @mock.patch('curtin.block.clear_holders.util')
179 @mock.patch('curtin.block.clear_holders.os')
180 @mock.patch('curtin.block.clear_holders.LOG')
181 @mock.patch('curtin.block.clear_holders.get_bcache_using_dev')
182 def test_shutdown_bcache_non_sysfs_device(self, mock_get_bcache, mock_log,
183 mock_os, mock_util,
184 mock_get_bcache_block):
185 device = "/dev/fakenull"
186 with self.assertRaises(ValueError):
187 clear_holders.shutdown_bcache(device)
188
189 self.assertEqual(0, len(mock_get_bcache.call_args_list))
190 self.assertEqual(0, len(mock_log.call_args_list))
191 self.assertEqual(0, len(mock_os.call_args_list))
192 self.assertEqual(0, len(mock_util.call_args_list))
193 self.assertEqual(0, len(mock_get_bcache_block.call_args_list))
194
195 @mock.patch('curtin.block.clear_holders.get_bcache_sys_path')
196 @mock.patch('curtin.block.clear_holders.util')
197 @mock.patch('curtin.block.clear_holders.os')
198 @mock.patch('curtin.block.clear_holders.LOG')
199 @mock.patch('curtin.block.clear_holders.get_bcache_using_dev')
200 def test_shutdown_bcache_no_device(self, mock_get_bcache, mock_log,
201 mock_os, mock_util,
202 mock_get_bcache_block):
203 device = "/sys/class/block/null"
204 mock_os.path.exists.return_value = False
205
206 clear_holders.shutdown_bcache(device)
207
208 self.assertEqual(1, len(mock_log.info.call_args_list))
209 self.assertEqual(1, len(mock_os.path.exists.call_args_list))
210 self.assertEqual(0, len(mock_get_bcache.call_args_list))
211 self.assertEqual(0, len(mock_util.call_args_list))
212 self.assertEqual(0, len(mock_get_bcache_block.call_args_list))
213
214 @mock.patch('curtin.block.clear_holders.get_bcache_sys_path')
215 @mock.patch('curtin.block.clear_holders.util')
216 @mock.patch('curtin.block.clear_holders.os')
217 @mock.patch('curtin.block.clear_holders.LOG')
218 @mock.patch('curtin.block.clear_holders.get_bcache_using_dev')
219 def test_shutdown_bcache_no_cset(self, mock_get_bcache, mock_log,
220 mock_os, mock_util,
221 mock_get_bcache_block):
222 device = "/sys/class/block/null"
223 mock_os.path.exists.side_effect = iter([
224 True, # backing device exists
225 False, # cset device not present (already removed)
226 True, # backing device (still) exists
227 ])
228 mock_get_bcache.return_value = '/sys/fs/bcache/fake'
229 mock_get_bcache_block.return_value = device + '/bcache'
230 mock_os.path.join.side_effect = os.path.join
231
232 clear_holders.shutdown_bcache(device)
233
234 self.assertEqual(2, len(mock_log.info.call_args_list))
235 self.assertEqual(3, len(mock_os.path.exists.call_args_list))
236 self.assertEqual(1, len(mock_get_bcache.call_args_list))
237 self.assertEqual(1, len(mock_get_bcache_block.call_args_list))
238 self.assertEqual(1, len(mock_util.write_file.call_args_list))
239 self.assertEqual(1, len(mock_util.wait_for_removal.call_args_list))
240
241 mock_get_bcache.assert_called_with(device)
242 mock_get_bcache_block.assert_called_with(device)
243 mock_util.write_file.assert_called_with(device + '/bcache/stop',
133 '1', mode=None)244 '1', mode=None)
245 retries = self.remove_retries
246 mock_util.wait_for_removal.assert_called_with(device, retries=retries)
247
248 @mock.patch('curtin.block.clear_holders.get_bcache_sys_path')
249 @mock.patch('curtin.block.clear_holders.util')
250 @mock.patch('curtin.block.clear_holders.os')
251 @mock.patch('curtin.block.clear_holders.LOG')
252 @mock.patch('curtin.block.clear_holders.get_bcache_using_dev')
253 def test_shutdown_bcache_delete_cset_and_backing(self, mock_get_bcache,
254 mock_log, mock_os,
255 mock_util,
256 mock_get_bcache_block):
257 device = "/sys/class/block/null"
258 mock_os.path.exists.side_effect = iter([
259 True, # backing device exists
260 True, # cset device not present (already removed)
261 True, # backing device (still) exists
262 ])
263 cset = '/sys/fs/bcache/fake'
264 mock_get_bcache.return_value = cset
265 mock_get_bcache_block.return_value = device + '/bcache'
266 mock_os.path.join.side_effect = os.path.join
267
268 clear_holders.shutdown_bcache(device)
269
270 self.assertEqual(2, len(mock_log.info.call_args_list))
271 self.assertEqual(3, len(mock_os.path.exists.call_args_list))
272 self.assertEqual(1, len(mock_get_bcache.call_args_list))
273 self.assertEqual(1, len(mock_get_bcache_block.call_args_list))
274 self.assertEqual(2, len(mock_util.write_file.call_args_list))
275 self.assertEqual(2, len(mock_util.wait_for_removal.call_args_list))
276
277 mock_get_bcache.assert_called_with(device)
278 mock_get_bcache_block.assert_called_with(device)
279 mock_util.write_file.assert_has_calls([
280 mock.call(cset + '/stop', '1', mode=None),
281 mock.call(device + '/bcache/stop', '1', mode=None)])
282 mock_util.wait_for_removal.assert_has_calls([
283 mock.call(cset, retries=self.remove_retries),
284 mock.call(device, retries=self.remove_retries)
285 ])
286
287 @mock.patch('curtin.block.clear_holders.get_bcache_sys_path')
288 @mock.patch('curtin.block.clear_holders.util')
289 @mock.patch('curtin.block.clear_holders.os')
290 @mock.patch('curtin.block.clear_holders.LOG')
291 @mock.patch('curtin.block.clear_holders.get_bcache_using_dev')
292 def test_shutdown_bcache_delete_cset_no_backing(self, mock_get_bcache,
293 mock_log, mock_os,
294 mock_util,
295 mock_get_bcache_block):
296 device = "/sys/class/block/null"
297 mock_os.path.exists.side_effect = iter([
298 True, # backing device exists
299 True, # cset device not present (already removed)
300 False, # backing device is removed with cset
301 ])
302 cset = '/sys/fs/bcache/fake'
303 mock_get_bcache.return_value = cset
304 mock_get_bcache_block.return_value = device + '/bcache'
305 mock_os.path.join.side_effect = os.path.join
306
307 clear_holders.shutdown_bcache(device)
308
309 self.assertEqual(2, len(mock_log.info.call_args_list))
310 self.assertEqual(3, len(mock_os.path.exists.call_args_list))
311 self.assertEqual(1, len(mock_get_bcache.call_args_list))
312 self.assertEqual(0, len(mock_get_bcache_block.call_args_list))
313 self.assertEqual(1, len(mock_util.write_file.call_args_list))
314 self.assertEqual(1, len(mock_util.wait_for_removal.call_args_list))
315
316 mock_get_bcache.assert_called_with(device)
317 mock_util.write_file.assert_has_calls([
318 mock.call(cset + '/stop', '1', mode=None),
319 ])
320 mock_util.wait_for_removal.assert_has_calls([
321 mock.call(cset, retries=self.remove_retries)
322 ])
134323
135 @mock.patch('curtin.block.clear_holders.LOG')324 @mock.patch('curtin.block.clear_holders.LOG')
136 @mock.patch('curtin.block.clear_holders.block.sys_block_path')325 @mock.patch('curtin.block.clear_holders.block.sys_block_path')
137326
=== modified file 'tests/unittests/test_util.py'
--- tests/unittests/test_util.py 2017-04-07 20:12:45 +0000
+++ tests/unittests/test_util.py 2017-04-24 17:53:29 +0000
@@ -614,4 +614,73 @@
614 except KeyError as e:614 except KeyError as e:
615 self.fail("unexpected key error raised: %s" % e)615 self.fail("unexpected key error raised: %s" % e)
616616
617
618class TestWaitForRemoval(TestCase):
619 def test_wait_for_removal_missing_path(self):
620 with self.assertRaises(ValueError):
621 util.wait_for_removal(None)
622
623 @mock.patch('curtin.util.time')
624 @mock.patch('curtin.util.os')
625 def test_wait_for_removal(self, mock_os, mock_time):
626 path = "/file/to/remove"
627 mock_os.path.exists.side_effect = iter([
628 True, # File is not yet removed
629 False, # File has been removed
630 ])
631
632 util.wait_for_removal(path)
633
634 self.assertEqual(2, len(mock_os.path.exists.call_args_list))
635 self.assertEqual(1, len(mock_time.sleep.call_args_list))
636 mock_os.path.exists.assert_has_calls([
637 mock.call(path),
638 mock.call(path),
639 ])
640 mock_time.sleep.assert_has_calls([
641 mock.call(1),
642 ])
643
644 @mock.patch('curtin.util.time')
645 @mock.patch('curtin.util.os')
646 def test_wait_for_removal_timesout(self, mock_os, mock_time):
647 path = "/file/to/remove"
648 mock_os.path.exists.return_value = True
649
650 with self.assertRaises(OSError):
651 util.wait_for_removal(path)
652
653 self.assertEqual(5, len(mock_os.path.exists.call_args_list))
654 self.assertEqual(4, len(mock_time.sleep.call_args_list))
655 mock_os.path.exists.assert_has_calls(5 * [mock.call(path)])
656 mock_time.sleep.assert_has_calls([
657 mock.call(1),
658 mock.call(3),
659 mock.call(5),
660 mock.call(7),
661 ])
662
663 @mock.patch('curtin.util.time')
664 @mock.patch('curtin.util.os')
665 def test_wait_for_removal_custom_retry(self, mock_os, mock_time):
666 path = "/file/to/remove"
667 timeout = 100
668 mock_os.path.exists.side_effect = iter([
669 True, # File is not yet removed
670 False, # File has been removed
671 ])
672
673 util.wait_for_removal(path, retries=[timeout])
674
675 self.assertEqual(2, len(mock_os.path.exists.call_args_list))
676 self.assertEqual(1, len(mock_time.sleep.call_args_list))
677 mock_os.path.exists.assert_has_calls([
678 mock.call(path),
679 mock.call(path),
680 ])
681 mock_time.sleep.assert_has_calls([
682 mock.call(timeout),
683 ])
684
685
617# vi: ts=4 expandtab syntax=python686# vi: ts=4 expandtab syntax=python
618687
=== modified file 'tests/vmtests/test_nvme.py'
--- tests/vmtests/test_nvme.py 2017-02-22 01:42:16 +0000
+++ tests/vmtests/test_nvme.py 2017-04-24 17:53:29 +0000
@@ -99,3 +99,80 @@
9999
100class ZestyTestNvme(relbase.zesty, TestNvmeAbs):100class ZestyTestNvme(relbase.zesty, TestNvmeAbs):
101 __test__ = True101 __test__ = True
102
103
104class TestNvmeBcacheAbs(VMBaseClass):
105 arch_skip = [
106 "s390x", # nvme is a pci device, no pci on s390x
107 ]
108 interactive = False
109 conf_file = "examples/tests/nvme_bcache.yaml"
110 extra_disks = ['10G']
111 nvme_disks = ['6G']
112 uefi = True
113 disk_to_check = [('sda', 1), ('sda', 2), ('sda', 3)]
114
115 collect_scripts = [textwrap.dedent("""
116 cd OUTPUT_COLLECT_D
117 ls /sys/class/ > sys_class
118 ls /sys/class/nvme/ > ls_nvme
119 ls /dev/nvme* > ls_dev_nvme
120 ls /dev/disk/by-dname/ > ls_dname
121 ls -al /dev/bcache/by-uuid/ > ls_bcache_by_uuid |:
122 blkid -o export /dev/vda > blkid_output_vda
123 blkid -o export /dev/vda1 > blkid_output_vda1
124 blkid -o export /dev/vda2 > blkid_output_vda2
125 bcache-super-show /dev/nvme0n1p1 > bcache_super_nvme0n1p1
126 ls /sys/fs/bcache > bcache_ls
127 cat /sys/block/bcache0/bcache/cache_mode > bcache_cache_mode
128 cat /proc/partitions > proc_partitions
129 ls -al /dev/disk/by-uuid/ > ls_uuid
130 cat /etc/fstab > fstab
131 mkdir -p /dev/disk/by-dname
132 ls /dev/disk/by-dname/ > ls_dname
133 find /etc/network/interfaces.d > find_interfacesd
134
135 v=""
136 out=$(apt-config shell v Acquire::HTTP::Proxy)
137 eval "$out"
138 echo "$v" > apt-proxy
139 """)]
140
141 def test_output_files_exist(self):
142 self.output_files_exist(["ls_nvme", "ls_dname", "ls_dev_nvme"])
143
144 def test_nvme_device_names(self):
145 ls_nvme = os.path.join(self.td.collect, 'ls_nvme')
146 # trusty and vivid do not have sys/class/nvme but
147 # nvme devices do work
148 if os.path.getsize(ls_nvme) > 0:
149 self.check_file_strippedline("ls_nvme", "nvme0")
150 else:
151 self.check_file_strippedline("ls_dev_nvme", "/dev/nvme0")
152 self.check_file_strippedline("ls_dev_nvme", "/dev/nvme0n1")
153 self.check_file_strippedline("ls_dev_nvme", "/dev/nvme0n1p1")
154
155 def test_bcache_output_files_exist(self):
156 self.output_files_exist(["bcache_super_nvme0n1p1", "bcache_ls",
157 "bcache_cache_mode"])
158
159 def test_bcache_status(self):
160 bcache_cset_uuid = None
161 bcache_super = self.load_collect_file("bcache_super_nvme0n1p1")
162 for line in bcache_super.splitlines():
163 if line != "" and line.split()[0] == "cset.uuid":
164 bcache_cset_uuid = line.split()[-1].rstrip()
165 self.assertIsNotNone(bcache_cset_uuid)
166 self.assertTrue(bcache_cset_uuid in
167 self.load_collect_file("bcache_ls").splitlines())
168
169 def test_bcache_cachemode(self):
170 self.check_file_regex("bcache_cache_mode", r"\[writeback\]")
171
172
173class XenialTestNvmeBcache(relbase.xenial, TestNvmeBcacheAbs):
174 __test__ = True
175
176
177class ZestyTestNvmeBcache(relbase.zesty, TestNvmeBcacheAbs):
178 __test__ = True

Subscribers

People subscribed via source and target branches