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
This commit is contained in:
Ian Wienand 2017-06-19 10:29:53 +10:00
parent a0f747932d
commit 5d5fa06e5c
3 changed files with 16 additions and 9 deletions

View File

@ -11,9 +11,10 @@
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations # License for the specific language governing permissions and limitations
# under the License. # under the License.
import logging
import random
import logging
import os
import random
from struct import pack from struct import pack
@ -173,6 +174,8 @@ class MBR(object):
return self return self
def __exit__(self, exc_type, exc_value, traceback): def __exit__(self, exc_type, exc_value, traceback):
self.image_fd.flush()
os.fsync(self.image_fd.fileno())
self.image_fd.close() self.image_fd.close()
def lba2chs(self, lba): def lba2chs(self, lba):

View File

@ -143,16 +143,13 @@ class Partitioning(PluginBase):
self.state['blockdev'][part_name] \ self.state['blockdev'][part_name] \
= {'device': partition_device_name} = {'device': partition_device_name}
# "saftey sync" to make sure the partitions are written
exec_sudo(["sync"])
# now all the partitions are created, get device-mapper to # now all the partitions are created, get device-mapper to
# mount them # mount them
if not os.path.exists("/.dockerenv"): if not os.path.exists("/.dockerenv"):
exec_sudo(["kpartx", "-avs", device_path]) exec_sudo(["kpartx", "-avs", device_path])
# We need to make sure udev finishes creating the device
# before continuting, so "udevadm settle". Otherwise later
# commands can fail with "file does not exist".
# XXX: "-s" (synchronous) to kpartx should avoid this,
# but experience shows it does not.
exec_sudo(["udevadm", "settle"])
else: else:
# If running inside Docker, make our nodes manually, # If running inside Docker, make our nodes manually,
# because udev will not be working. kpartx cannot run in # because udev will not be working. kpartx cannot run in

View File

@ -12,6 +12,7 @@
import fixtures import fixtures
import logging import logging
import mock
import os import os
import subprocess import subprocess
@ -62,11 +63,17 @@ class TestMBR(tb.TestBase):
logger.info("Running command: %s", self.partx_args) logger.info("Running command: %s", self.partx_args)
return subprocess.check_output(self.partx_args).decode("ascii") return subprocess.check_output(self.partx_args).decode("ascii")
def test_one_ext_partition(self): @mock.patch('os.fsync', wraps=os.fsync)
def test_one_ext_partition(self, mock_os_fsync):
"""Creates one partition and check correctness with partx.""" """Creates one partition and check correctness with partx."""
with MBR(self.image_path, TestMBR.disk_size_1G, 1024 * 1024) as mbr: with MBR(self.image_path, TestMBR.disk_size_1G, 1024 * 1024) as mbr:
mbr.add_partition(False, False, TestMBR.disk_size_10M, 0x83) mbr.add_partition(False, False, TestMBR.disk_size_10M, 0x83)
# the exit handler of MBR should have synced the raw device
# before exit
mock_os_fsync.assert_called()
output = self._run_partx(self.image_path) output = self._run_partx(self.image_path)
self.assertEqual( self.assertEqual(
"1 2048 2097151 0xf 0x0 dos\n" "1 2048 2097151 0xf 0x0 dos\n"