1
=== modified file 'curtin/block/clear_holders.py'
2
--- curtin/block/clear_holders.py	2017-05-24 18:42:40 +0000
3
+++ curtin/block/clear_holders.py	2017-10-13 17:48:44 +0000
4
@@ -93,6 +93,22 @@
5
93
    return path
93
    return path
6
94
94
7
95
95
8
96
def maybe_stop_bcache_device(device):
9
97
    """Attempt to stop the provided device_path or raise unexpected errors."""
10
98
    bcache_stop = os.path.join(device, 'stop')
11
99
    try:
12
100
        util.write_file(bcache_stop, '1', mode=None)
13
101
    except (IOError, OSError) as e:
14
102
        # Note: if we get a write failure and the exception is that the
15
103
        # file is not found; we log this expected behavior; any other
16
104
        # exception is raised
17
105
        if e.errno == errno.ENOENT:
18
106
            LOG.debug('bcache stop file %s missing, device removed: %s',
19
107
                      bcache_stop, e)
20
108
        else:
21
109
            raise e
22
110
23
111
24
96
def shutdown_bcache(device):
112
def shutdown_bcache(device):
25
97
    """
113
    """
26
98
    Shut down bcache for specified bcache device
114
    Shut down bcache for specified bcache device
27
@@ -132,8 +148,7 @@
28
132
                 os.path.basename(bcache_cache_sysfs))
148
                 os.path.basename(bcache_cache_sysfs))
29
133
    else:
149
    else:
30
134
        LOG.info('stopping bcache cacheset at: %s', bcache_cache_sysfs)
150
        LOG.info('stopping bcache cacheset at: %s', bcache_cache_sysfs)
33
135
        util.write_file(os.path.join(bcache_cache_sysfs, 'stop'),
151
        maybe_stop_bcache_device(bcache_cache_sysfs)
32
136
                        '1', mode=None)
34
137
        try:
152
        try:
35
138
            util.wait_for_removal(bcache_cache_sysfs, retries=removal_retries)
153
            util.wait_for_removal(bcache_cache_sysfs, retries=removal_retries)
36
139
        except OSError:
154
        except OSError:
37
@@ -162,8 +177,7 @@
38
162
        return
177
        return
39
163
    else:
178
    else:
40
164
        LOG.info('stopping bcache backing device at: %s', bcache_block_sysfs)
179
        LOG.info('stopping bcache backing device at: %s', bcache_block_sysfs)
43
165
        util.write_file(os.path.join(bcache_block_sysfs, 'stop'),
180
        maybe_stop_bcache_device(bcache_block_sysfs)
42
166
                        '1', mode=None)
44
167
        try:
181
        try:
45
168
            # wait for them all to go away
182
            # wait for them all to go away
46
169
            for dev in [device, bcache_block_sysfs] + slave_paths:
183
            for dev in [device, bcache_block_sysfs] + slave_paths:
47
170
184
48
=== modified file 'tests/unittests/test_clear_holders.py'
49
--- tests/unittests/test_clear_holders.py	2017-08-02 20:55:03 +0000
50
+++ tests/unittests/test_clear_holders.py	2017-10-13 17:48:44 +0000
51
@@ -1,3 +1,4 @@
52
1
import errno
53
1
import mock
2
import mock
54
2
import os
3
import os
55
3
import textwrap
4
import textwrap
56
@@ -329,6 +330,52 @@
57
329
            mock.call(cset, retries=self.remove_retries)
330
            mock.call(cset, retries=self.remove_retries)
58
330
        ])
331
        ])
59
331
332
60
333
    # test bcache shutdown with 'stop' sysfs write failure
61
334
    @mock.patch('curtin.block.clear_holders.udev.udevadm_settle')
62
335
    @mock.patch('curtin.block.clear_holders.get_bcache_sys_path')
63
336
    @mock.patch('curtin.block.clear_holders.util')
64
337
    @mock.patch('curtin.block.clear_holders.os')
65
338
    @mock.patch('curtin.block.clear_holders.LOG')
66
339
    @mock.patch('curtin.block.clear_holders.get_bcache_using_dev')
67
340
    def test_shutdown_bcache_stop_sysfs_write_fails(self, mock_get_bcache,
68
341
                                                    mock_log, mock_os,
69
342
                                                    mock_util,
70
343
                                                    mock_get_bcache_block,
71
344
                                                    mock_udevadm_settle):
72
345
        """Test writes sysfs write failures pass if file not present"""
73
346
        device = "/sys/class/block/null"
74
347
        mock_os.path.exists.side_effect = iter([
75
348
                True,   # backing device exists
76
349
                True,   # cset device not present (already removed)
77
350
                False,  # backing device is removed with cset
78
351
                False,  # bcache/stop sysfs is missing (already removed)
79
352
        ])
80
353
        cset = '/sys/fs/bcache/fake'
81
354
        mock_get_bcache.return_value = cset
82
355
        mock_get_bcache_block.return_value = device + '/bcache'
83
356
        mock_os.path.join.side_effect = os.path.join
84
357
85
358
        # make writes to sysfs fail
86
359
        mock_util.write_file.side_effect = IOError(errno.ENOENT,
87
360
                                                   "File not found")
88
361
89
362
        clear_holders.shutdown_bcache(device)
90
363
91
364
        self.assertEqual(2, len(mock_log.info.call_args_list))
92
365
        self.assertEqual(3, len(mock_os.path.exists.call_args_list))
93
366
        self.assertEqual(1, len(mock_get_bcache.call_args_list))
94
367
        self.assertEqual(1, len(mock_get_bcache_block.call_args_list))
95
368
        self.assertEqual(1, len(mock_util.write_file.call_args_list))
96
369
        self.assertEqual(1, len(mock_util.wait_for_removal.call_args_list))
97
370
98
371
        mock_get_bcache.assert_called_with(device, strict=False)
99
372
        mock_util.write_file.assert_has_calls([
100
373
            mock.call(cset + '/stop', '1', mode=None),
101
374
        ])
102
375
        mock_util.wait_for_removal.assert_has_calls([
103
376
            mock.call(cset, retries=self.remove_retries)
104
377
        ])
105
378
106
332
    @mock.patch('curtin.block.clear_holders.LOG')
379
    @mock.patch('curtin.block.clear_holders.LOG')
107
333
    @mock.patch('curtin.block.clear_holders.block.sys_block_path')
380
    @mock.patch('curtin.block.clear_holders.block.sys_block_path')
108
334
    @mock.patch('curtin.block.clear_holders.lvm')
381
    @mock.patch('curtin.block.clear_holders.lvm')
109
@@ -531,6 +578,41 @@
110
531
        for tree, result in test_trees_and_results:
578
        for tree, result in test_trees_and_results:
111
532
            self.assertEqual(clear_holders.format_holders_tree(tree), result)
579
            self.assertEqual(clear_holders.format_holders_tree(tree), result)
112
533
580
113
581
    @mock.patch('curtin.block.clear_holders.util.write_file')
114
582
    def test_maybe_stop_bcache_device_raises_errors(self, m_write_file):
115
583
        """Unexpected exceptions are raised by maybe_stop_bcache_device."""
116
584
        m_write_file.side_effect = OSError(errno.EAGAIN, 'Unexpected errno')
117
585
        with self.assertRaises(OSError) as cm:
118
586
            clear_holders.maybe_stop_bcache_device('does/not/matter')
119
587
        self.assertEqual('[Errno 11] Unexpected errno', str(cm.exception))
120
588
        self.assertEqual(
121
589
            mock.call('does/not/matter/stop', '1', mode=None),
122
590
            m_write_file.call_args)
123
591
124
592
    @mock.patch('curtin.block.clear_holders.LOG')
125
593
    @mock.patch('curtin.block.clear_holders.util.write_file')
126
594
    def test_maybe_stop_bcache_device_handles_oserror(self, m_write_file,
127
595
                                                      m_log):
128
596
        """When OSError.NOENT is raised, log the condition and move on."""
129
597
        m_write_file.side_effect = OSError(errno.ENOENT, 'Expected oserror')
130
598
        clear_holders.maybe_stop_bcache_device('does/not/matter')
131
599
        self.assertEqual(
132
600
            'bcache stop file %s missing, device removed: %s',
133
601
            m_log.debug.call_args[0][0])
134
602
        self.assertEqual('does/not/matter/stop', m_log.debug.call_args[0][1])
135
603
136
604
    @mock.patch('curtin.block.clear_holders.LOG')
137
605
    @mock.patch('curtin.block.clear_holders.util.write_file')
138
606
    def test_maybe_stop_bcache_device_handles_ioerror(self, m_write_file,
139
607
                                                      m_log):
140
608
        """When IOError.NOENT is raised, log the condition and move on."""
141
609
        m_write_file.side_effect = IOError(errno.ENOENT, 'Expected ioerror')
142
610
        clear_holders.maybe_stop_bcache_device('does/not/matter')
143
611
        self.assertEqual(
144
612
            'bcache stop file %s missing, device removed: %s',
145
613
            m_log.debug.call_args[0][0])
146
614
        self.assertEqual('does/not/matter/stop', m_log.debug.call_args[0][1])
147
615
148
534
    def test_get_holder_types(self):
616
    def test_get_holder_types(self):
149
535
        """test clear_holders.get_holder_types"""
617
        """test clear_holders.get_holder_types"""
150
536
        test_trees_and_results = [
618
        test_trees_and_results = [
Status:	Merged
Merged at revision:	535
Proposed branch:	lp:~raharper/curtin/trunk.fix-bcache-sysfs-write-failures-on-remove
Merge into:	lp:~curtin-dev/curtin/trunk
Diff against target:	150 lines (+100/-4) 2 files modified curtin/block/clear_holders.py (+18/-4) tests/unittests/test_clear_holders.py (+82/-0)
To merge this branch:	bzr merge lp:~raharper/curtin/trunk.fix-bcache-sysfs-write-failures-on-remove
Related bugs:	Link a bug report
Reviewer	Review Type	Date Requested	Status
Server Team CI bot	continuous-integration		Approve on 2017-10-13
Chad Smith			Approve on 2017-10-13
Scott Moser (community)		2017-09-14	Needs Information on 2017-10-12
Review via email: mp+330758@code.launchpad.net