From 653b59a09d93af736ac02733de05045097bb2b3d Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Wed, 9 Jan 2019 16:30:47 +0100 Subject: [PATCH] Run sync and partprobe after adding a configdrive partition We're hitting a problem in the CI where /dev/sda2 does not exist after being created. The logs suggest running partprobe may fix the problem, so let's try it. With this, we can also encounter a case where there refresh will fail because the partition was created using fsync calls to force the write of the partition table. However, filesystem VFS layer does impact this behavior, and fsync on files in a ramdisk is a noop, so we must force everything to be flushed to disk before attempting to execute partprobe. Also run 'sgdisk -v' after partitioning to leave some traces in the logs for debugging when something goes wrong. Story: #2004744 Task: #28815 Change-Id: I9db48a3462422749290bbb887c14816734ab0478 Depends-On: I8d6f8c99e1ab5f26e3537630d0f6086e02a2b1ec --- ironic_lib/disk_utils.py | 19 +++++++++++++++++++ ironic_lib/tests/test_disk_utils.py | 24 ++++++++++++++++++++---- 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/ironic_lib/disk_utils.py b/ironic_lib/disk_utils.py index a2e3134b..d53df175 100644 --- a/ironic_lib/disk_utils.py +++ b/ironic_lib/disk_utils.py @@ -874,6 +874,25 @@ def create_config_drive_partition(node_uuid, device, configdrive): utils.execute('parted', '-a', 'optimal', '-s', '--', device, 'mkpart', 'primary', 'fat32', startlimit, endlimit, run_as_root=True) + # Parted uses fsync to tell the kernel to sync file io + # however on ramdisks in ramfs, this is an explicit no-op. + # Explicitly call sync so when the the kernel attempts to read + # the partition table from disk, it is less likely that the write + # is still in buffer cache pending write to disk. + LOG.debug('Explicitly calling sync to force buffer/cache flush.') + utils.execute('sync') + # Make sure any additions to the partitioning are reflected in the + # kernel. + LOG.debug('Waiting until udev event queue is empty') + utils.execute('udevadm', 'settle') + try: + utils.execute('partprobe', device, run_as_root=True, + attempts=CONF.disk_utils.partprobe_attempts) + # Also verify that the partitioning is correct now. + utils.execute('sgdisk', '-v', device, run_as_root=True) + except processutils.ProcessExecutionError as exc: + LOG.warning('Failed to verify GPT partitioning after creating ' + 'the configdrive partition: %s', exc) upd_parts = set(part['number'] for part in list_partitions(device)) new_part = set(upd_parts) - set(cur_parts) diff --git a/ironic_lib/tests/test_disk_utils.py b/ironic_lib/tests/test_disk_utils.py index 2c311b47..e138601e 100644 --- a/ironic_lib/tests/test_disk_utils.py +++ b/ironic_lib/tests/test_disk_utils.py @@ -1246,6 +1246,11 @@ class WholeDiskConfigDriveTestCases(base.IronicLibTestCase): mock_execute.assert_has_calls([ mock.call('sgdisk', '-n', '0:-64MB:0', self.dev, run_as_root=True), + mock.call('sync'), + mock.call('udevadm', 'settle'), + mock.call('partprobe', self.dev, attempts=10, run_as_root=True), + mock.call('sgdisk', '-v', self.dev, run_as_root=True), + mock.call('udevadm', 'settle'), mock.call('test', '-e', expected_part, attempts=15, check_exit_code=[0], delay_on_retry=True) @@ -1347,6 +1352,10 @@ class WholeDiskConfigDriveTestCases(base.IronicLibTestCase): '-0', run_as_root=True) mock_execute.assert_has_calls([ parted_call, + mock.call('sync'), + mock.call('udevadm', 'settle'), + mock.call('partprobe', self.dev, attempts=10, run_as_root=True), + mock.call('sgdisk', '-v', self.dev, run_as_root=True), mock.call('udevadm', 'settle'), mock.call('test', '-e', expected_part, attempts=15, check_exit_code=[0], delay_on_retry=True) @@ -1443,10 +1452,17 @@ class WholeDiskConfigDriveTestCases(base.IronicLibTestCase): self.node_uuid, self.dev, config_url) mock_get_configdrive.assert_called_with(config_url, self.node_uuid) - mock_execute.assert_called_with('parted', '-a', 'optimal', '-s', '--', - self.dev, 'mkpart', 'primary', - 'fat32', '-64MiB', '-0', - run_as_root=True) + mock_execute.assert_has_calls([ + mock.call('parted', '-a', 'optimal', '-s', '--', + self.dev, 'mkpart', 'primary', + 'fat32', '-64MiB', '-0', + run_as_root=True), + mock.call('sync'), + mock.call('udevadm', 'settle'), + mock.call('partprobe', self.dev, attempts=10, run_as_root=True), + mock.call('sgdisk', '-v', self.dev, run_as_root=True), + ]) + self.assertEqual(2, mock_list_partitions.call_count) mock_fix_gpt_partition.assert_called_with(self.dev, self.node_uuid) mock_is_disk_gpt.assert_called_with(self.dev, self.node_uuid)