summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorIan Wienand <iwienand@redhat.com>2017-06-19 10:29:53 +1000
committerIan Wienand <iwienand@redhat.com>2017-06-19 17:13:36 +1000
commit5d5fa06e5c53ca8dc857d1700b57c2336ac62db1 (patch)
treea751da8fe0fb71bda22f6bd3b9b9b868c81d8fa0
parenta0f747932d0ef321355f62205d2e47b93ddc06dd (diff)
Sync after writing partition table
We introduced the "settle" in I90103b59357edebbac7a641e8980cb282d37561b thinking that maybe kpartx had not finished writing the partition. This probably wasn't a bad first assumption, since we used to have this -- but is seems insufficient. The other failiure here seems to be if kpartx hasn't actually seen the updated partition table in the image, so it has correctly (in it's mind) not mounted the partition. Looking at strace of fdisk run manually on a loopback, it will do a fsync on the raw device after writing and then a global sync as it exits. This replicates this; we flush and fsync in mbr.py in the exit handler after writing the partition, before closing the file (i've updated one of the unit tests to double-check the call). In the partitioning.py caller we execute a sync call too. Since it does seem unlikely the "-s" option of kpartx is not working, I've removed the udev settle work-around too. Change-Id: Ia77a0ffe4c76854b326ed76490479d9c691b49aa Partial-Bug: #1698337
Notes
Notes (review): Code-Review+2: yolanda.robla <yroblamo@redhat.com> Workflow+1: yolanda.robla <yroblamo@redhat.com> Verified+2: Jenkins Submitted-by: Jenkins Submitted-at: Mon, 19 Jun 2017 16:49:31 +0000 Reviewed-on: https://review.openstack.org/475203 Project: openstack/diskimage-builder Branch: refs/heads/master
-rw-r--r--diskimage_builder/block_device/level1/mbr.py5
-rw-r--r--diskimage_builder/block_device/level1/partitioning.py9
-rw-r--r--diskimage_builder/block_device/tests/test_mbr.py9
3 files changed, 15 insertions, 8 deletions
diff --git a/diskimage_builder/block_device/level1/mbr.py b/diskimage_builder/block_device/level1/mbr.py
index 8fab1e8..6b3f87f 100644
--- a/diskimage_builder/block_device/level1/mbr.py
+++ b/diskimage_builder/block_device/level1/mbr.py
@@ -11,10 +11,11 @@
11# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the 11# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
12# License for the specific language governing permissions and limitations 12# License for the specific language governing permissions and limitations
13# under the License. 13# under the License.
14
14import logging 15import logging
16import os
15import random 17import random
16 18
17
18from struct import pack 19from struct import pack
19 20
20 21
@@ -173,6 +174,8 @@ class MBR(object):
173 return self 174 return self
174 175
175 def __exit__(self, exc_type, exc_value, traceback): 176 def __exit__(self, exc_type, exc_value, traceback):
177 self.image_fd.flush()
178 os.fsync(self.image_fd.fileno())
176 self.image_fd.close() 179 self.image_fd.close()
177 180
178 def lba2chs(self, lba): 181 def lba2chs(self, lba):
diff --git a/diskimage_builder/block_device/level1/partitioning.py b/diskimage_builder/block_device/level1/partitioning.py
index 926207e..e8a5598 100644
--- a/diskimage_builder/block_device/level1/partitioning.py
+++ b/diskimage_builder/block_device/level1/partitioning.py
@@ -143,16 +143,13 @@ class Partitioning(PluginBase):
143 self.state['blockdev'][part_name] \ 143 self.state['blockdev'][part_name] \
144 = {'device': partition_device_name} 144 = {'device': partition_device_name}
145 145
146 # "saftey sync" to make sure the partitions are written
147 exec_sudo(["sync"])
148
146 # now all the partitions are created, get device-mapper to 149 # now all the partitions are created, get device-mapper to
147 # mount them 150 # mount them
148 if not os.path.exists("/.dockerenv"): 151 if not os.path.exists("/.dockerenv"):
149 exec_sudo(["kpartx", "-avs", device_path]) 152 exec_sudo(["kpartx", "-avs", device_path])
150 # We need to make sure udev finishes creating the device
151 # before continuting, so "udevadm settle". Otherwise later
152 # commands can fail with "file does not exist".
153 # XXX: "-s" (synchronous) to kpartx should avoid this,
154 # but experience shows it does not.
155 exec_sudo(["udevadm", "settle"])
156 else: 153 else:
157 # If running inside Docker, make our nodes manually, 154 # If running inside Docker, make our nodes manually,
158 # because udev will not be working. kpartx cannot run in 155 # because udev will not be working. kpartx cannot run in
diff --git a/diskimage_builder/block_device/tests/test_mbr.py b/diskimage_builder/block_device/tests/test_mbr.py
index 953dc17..606b943 100644
--- a/diskimage_builder/block_device/tests/test_mbr.py
+++ b/diskimage_builder/block_device/tests/test_mbr.py
@@ -12,6 +12,7 @@
12 12
13import fixtures 13import fixtures
14import logging 14import logging
15import mock
15import os 16import os
16import subprocess 17import subprocess
17 18
@@ -62,11 +63,17 @@ class TestMBR(tb.TestBase):
62 logger.info("Running command: %s", self.partx_args) 63 logger.info("Running command: %s", self.partx_args)
63 return subprocess.check_output(self.partx_args).decode("ascii") 64 return subprocess.check_output(self.partx_args).decode("ascii")
64 65
65 def test_one_ext_partition(self): 66 @mock.patch('os.fsync', wraps=os.fsync)
67 def test_one_ext_partition(self, mock_os_fsync):
66 """Creates one partition and check correctness with partx.""" 68 """Creates one partition and check correctness with partx."""
67 69
68 with MBR(self.image_path, TestMBR.disk_size_1G, 1024 * 1024) as mbr: 70 with MBR(self.image_path, TestMBR.disk_size_1G, 1024 * 1024) as mbr:
69 mbr.add_partition(False, False, TestMBR.disk_size_10M, 0x83) 71 mbr.add_partition(False, False, TestMBR.disk_size_10M, 0x83)
72
73 # the exit handler of MBR should have synced the raw device
74 # before exit
75 mock_os_fsync.assert_called()
76
70 output = self._run_partx(self.image_path) 77 output = self._run_partx(self.image_path)
71 self.assertEqual( 78 self.assertEqual(
72 "1 2048 2097151 0xf 0x0 dos\n" 79 "1 2048 2097151 0xf 0x0 dos\n"