curtin-dev team mailing list archive
-
curtin-dev team
-
Mailing list archive
-
Message #02880
[Merge] ~dbungert/curtin:compat into curtin:master
Dan Bungert has proposed merging ~dbungert/curtin:compat into curtin:master.
Commit message:
add compat module, and use for sfdisk/util-linux quirks
Requested reviews:
Michael Hudson-Doyle (mwhudson)
Server Team CI bot (server-team-bot): continuous-integration
For more details, see:
https://code.launchpad.net/~dbungert/curtin/+git/curtin/+merge/443511
--
Your team curtin developers is subscribed to branch curtin:master.
diff --git a/curtin/block/__init__.py b/curtin/block/__init__.py
index 2f69bd3..86251d9 100644
--- a/curtin/block/__init__.py
+++ b/curtin/block/__init__.py
@@ -1042,9 +1042,21 @@ def check_dos_signature(device):
# this signature must be at 0x1fe
# https://en.wikipedia.org/wiki/Master_boot_record#Sector_layout
devname = dev_path(path_to_kname(device))
- return (is_block_device(devname) and util.file_size(devname) >= 0x200 and
- (util.load_file(devname, decode=False, read_len=2, offset=0x1fe) ==
- b'\x55\xAA'))
+ if not is_block_device(devname):
+ return False
+ try:
+ # Some older series have the extended partition block device but return
+ # ENXIO when attempting to read it.
+ file_size = util.file_size(devname)
+ except OSError as ose:
+ if ose.errno == errno.ENXIO:
+ return False
+ else:
+ raise
+ if file_size < 0x200:
+ return False
+ signature = util.load_file(devname, decode=False, read_len=2, offset=0x1fe)
+ return signature == b'\x55\xAA'
def check_efi_signature(device):
diff --git a/curtin/commands/block_meta.py b/curtin/commands/block_meta.py
index 5c5a7b0..6ce8440 100644
--- a/curtin/commands/block_meta.py
+++ b/curtin/commands/block_meta.py
@@ -1,7 +1,7 @@
# This file is part of curtin. See LICENSE file for copyright and license info.
from collections import OrderedDict, namedtuple
-from curtin import (block, config, paths, util)
+from curtin import (block, compat, config, paths, util)
from curtin.block import schemas
from curtin.block import (bcache, clear_holders, dasd, iscsi, lvm, mdadm, mkfs,
multipath, zfs)
@@ -653,7 +653,6 @@ DEVS = set()
def image_handler(info, storage_config, context):
path = info['path']
size = int(util.human2bytes(info['size']))
- sector_size = str(int(util.human2bytes(info.get('sector_size', 512))))
if info.get('preserve', False):
actual_size = os.stat(path).st_size
if size != actual_size:
@@ -670,10 +669,14 @@ def image_handler(info, storage_config, context):
if os.path.exists(path):
os.unlink(path)
raise
+
+ cmd = ['losetup', '--show', '--find', path]
+ sector_size = int(util.human2bytes(info.get('sector_size', 512)))
+ if sector_size != 512:
+ compat.supports_large_sectors(fatal=True)
+ cmd.extend(('--sector-size', str(sector_size)))
try:
- dev = util.subp([
- 'losetup', '--show', '--sector-size', sector_size, '--find', path],
- capture=True)[0].strip()
+ dev = util.subp(cmd, capture=True)[0].strip()
except BaseException:
if os.path.exists(path) and not info.get('preserve'):
os.unlink(path)
@@ -858,17 +861,28 @@ def calc_dm_partition_info(partition_kname):
def calc_partition_info(partition_kname, logical_block_size_bytes):
+ p_size_sec = 0
+ p_start_sec = 0
if partition_kname.startswith('dm-'):
p_start, p_size = calc_dm_partition_info(partition_kname)
else:
pdir = block.sys_block_path(partition_kname)
p_size = int(util.load_file(os.path.join(pdir, "size")))
p_start = int(util.load_file(os.path.join(pdir, "start")))
+ if not all([p_size, p_start]):
+ # if sysfs reported a 0, let's try sfdisk
+ sfdisk_info = block.sfdisk_info(partition_kname)
+ part_path = block.kname_to_path(partition_kname)
+ part_info = block.get_partition_sfdisk_info(part_path, sfdisk_info)
+ p_size_sec = part_info['size']
+ p_start_sec = part_info['start']
# NB: sys/block/X/{size,start} and dmsetup output are both always
# in 512b sectors
- p_size_sec = p_size * 512 // logical_block_size_bytes
- p_start_sec = p_start * 512 // logical_block_size_bytes
+ if p_size_sec == 0:
+ p_size_sec = p_size * 512 // logical_block_size_bytes
+ if p_start_sec == 0:
+ p_start_sec = p_start * 512 // logical_block_size_bytes
LOG.debug("calc_partition_info: %s size_sectors=%s start_sectors=%s",
partition_kname, p_size_sec, p_start_sec)
diff --git a/curtin/commands/block_meta_v2.py b/curtin/commands/block_meta_v2.py
index 7f5ebce..570da83 100644
--- a/curtin/commands/block_meta_v2.py
+++ b/curtin/commands/block_meta_v2.py
@@ -8,7 +8,7 @@ from typing import (
import attr
-from curtin import (block, util)
+from curtin import (block, compat, util)
from curtin.commands.block_meta import (
_get_volume_fstype,
disk_handler as disk_handler_v1,
@@ -171,9 +171,10 @@ class SFDiskPartTable:
def apply(self, device):
sfdisk_script = self.render()
LOG.debug("sfdisk input:\n---\n%s\n---\n", sfdisk_script)
- util.subp(
- ['sfdisk', '--no-tell-kernel', '--no-reread', device],
- data=sfdisk_script.encode('ascii'))
+ cmd = ['sfdisk', '--no-reread', device]
+ if compat.supports_sfdisk_no_tell_kernel():
+ cmd.append('--no-tell-kernel')
+ util.subp(cmd, data=sfdisk_script.encode('ascii'))
util.subp(['partprobe', device])
# sfdisk and partprobe (as invoked here) use ioctls to inform the
# kernel that the partition table has changed so it can add and remove
diff --git a/curtin/compat.py b/curtin/compat.py
new file mode 100644
index 0000000..c20b650
--- /dev/null
+++ b/curtin/compat.py
@@ -0,0 +1,38 @@
+# This file is part of curtin. See LICENSE file for copyright and license info.
+
+import re
+
+from curtin import util
+
+
+def _get_util_linux_ver():
+ line = util.subp(['losetup', '--version'], capture=True)[0].strip()
+ m = re.fullmatch(r'losetup from util-linux ([\d.]+)', line)
+ if m is None:
+ return None
+ return m.group(1)
+
+
+def _check_util_linux_ver(ver, label='', fatal=False):
+ ul_ver = _get_util_linux_ver()
+ result = ul_ver is not None and ul_ver >= ver
+ if not result and fatal:
+ raise RuntimeError(
+ 'this system lacks the required {} support'.format(label))
+ return result
+
+
+def supports_large_sectors(fatal=False):
+ # Known requirements:
+ # * Kernel 4.14+
+ # * Minimum supported things have a higher kernel, so skip that check
+ # * util-linux 2.30+
+ # * xenial has 2.27.1, bionic has 2.31.1
+ # However, see also this, which suggests using 2.37.1:
+ # https://lore.kernel.org/lkml/20210615084259.yj5pmyjonfqcg7lg@xxxxxxxxxxx/
+ return _check_util_linux_ver('2.37.1', 'large sector', fatal)
+
+
+def supports_sfdisk_no_tell_kernel(fatal=False):
+ # Needs util-linux 2.29+
+ return _check_util_linux_ver('2.29', 'sfdisk', fatal)
diff --git a/requirements.txt b/requirements.txt
index d8d1a3d..71a2637 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,3 +1,6 @@
+pip
+wheel
+virtualenv
pyudev
pyyaml
oauthlib
diff --git a/tests/integration/test_block_meta.py b/tests/integration/test_block_meta.py
index 0afa9fd..e571cdc 100644
--- a/tests/integration/test_block_meta.py
+++ b/tests/integration/test_block_meta.py
@@ -8,12 +8,17 @@ import os
from parameterized import parameterized
import re
import sys
+import tempfile
from typing import Optional
from unittest import skipIf
import yaml
+<<<<<<< tests/integration/test_block_meta.py
from curtin import block, distro, log, udev, util
+=======
+from curtin import block, compat, distro, log, udev, util
+>>>>>>> tests/integration/test_block_meta.py
from curtin.commands.block_meta import _get_volume_fstype
from curtin.commands.block_meta_v2 import ONE_MIB_BYTES
@@ -27,11 +32,10 @@ class IntegrationTestCase(CiTestCase):
@contextlib.contextmanager
def loop_dev(image, sector_size=512):
- dev = util.subp([
- 'losetup',
- '--show', '--find', '--sector-size', str(sector_size),
- image,
- ], capture=True, decode='ignore')[0].strip()
+ cmd = ['losetup', '--show', '--find', image]
+ if sector_size != 512:
+ cmd.extend(('--sector-size', str(sector_size)))
+ dev = util.subp(cmd, capture=True, decode='ignore')[0].strip()
util.subp(['partprobe', dev])
try:
udev.udevadm_trigger([dev])
@@ -118,7 +122,7 @@ def summarize_partitions(dev):
parts = []
ptable_json = util.subp(['sfdisk', '-J', dev], capture=True)[0]
ptable = json.loads(ptable_json)['partitiontable']
- sectorsize = ptable['sectorsize']
+ sectorsize = ptable.get('sectorsize', 512)
assert dev == ptable['device']
sysfs_data = block.sysfs_partition_data(dev)
for part in ptable['partitions']:
@@ -257,7 +261,25 @@ class TestBlockMeta(IntegrationTestCase):
'-c', config_path, 'block-meta', '--testmode', 'custom',
*args,
]
- util.subp(cmd, env=cmd_env, **kwargs)
+
+ # Set debug=True to halt the integration test and run curtin manually,
+ # with the integration tests having setup the environment for you.
+ # Run tests with pytest-3 -s option.
+ if kwargs.pop('debug', False):
+ with tempfile.NamedTemporaryFile(mode='w', delete=False) as fp:
+ fp.write('#!/bin/bash\n')
+ for k, v in cmd_env.items():
+ fp.write('export {}="{}"\n'.format(k, v))
+ fp.write('export PYTHONPATH="{}"\n'.format(os.getcwd()))
+ fp.write(' '.join(cmd) + '\n')
+ os.chmod(fp.name, 0o700)
+ print()
+ print('The integration test is paused.')
+ print('Use script {} to run curtin manually.'.format(fp.name))
+ breakpoint()
+ os.unlink(fp.name)
+ else:
+ util.subp(cmd, env=cmd_env, **kwargs)
def _test_default_offsets(self, ptable, version, sector_size=512):
psize = 40 << 20
@@ -311,15 +333,19 @@ class TestBlockMeta(IntegrationTestCase):
def test_default_offsets_msdos_v2(self):
self._test_default_offsets('msdos', 2)
+ @skipIf(not compat.supports_large_sectors(), 'test is for large sectors')
def test_default_offsets_gpt_v1_4k(self):
self._test_default_offsets('gpt', 1, 4096)
+ @skipIf(not compat.supports_large_sectors(), 'test is for large sectors')
def test_default_offsets_msdos_v1_4k(self):
self._test_default_offsets('msdos', 1, 4096)
+ @skipIf(not compat.supports_large_sectors(), 'test is for large sectors')
def test_default_offsets_gpt_v2_4k(self):
self._test_default_offsets('gpt', 2, 4096)
+ @skipIf(not compat.supports_large_sectors(), 'test is for large sectors')
def test_default_offsets_msdos_v2_4k(self):
self._test_default_offsets('msdos', 2, 4096)
@@ -424,8 +450,17 @@ class TestBlockMeta(IntegrationTestCase):
PartData(number=6, offset=13 << 20, size=10 << 20),
])
- p1kname = block.partition_kname(block.path_to_kname(dev), 1)
- self.assertTrue(block.is_extended_partition('/dev/' + p1kname))
+ if distro.lsb_release()['release'] >= '20.04':
+ p1kname = block.partition_kname(block.path_to_kname(dev), 1)
+ self.assertTrue(block.is_extended_partition('/dev/' + p1kname))
+ else:
+ # on Bionic and earlier, the block device for the extended
+ # partition is not functional, so attempting to verify it is
+ # expected to fail. So just read the value directly from the
+ # expected signature location.
+ signature = util.load_file(dev, decode=False,
+ read_len=2, offset=0x1001fe)
+ self.assertEqual(b'\x55\xAA', signature)
def test_logical_v1(self):
self._test_logical(1)
@@ -716,6 +751,8 @@ class TestBlockMeta(IntegrationTestCase):
PartData(number=6, offset=13 << 20, size=20 << 20),
])
+ @skipIf(distro.lsb_release()['release'] < '20.04',
+ 'old lsblk will not list info about extended partitions')
def test_resize_extended(self):
img = self.tmp_path('image.img')
config = StorageConfigBuilder(version=2)
@@ -865,6 +902,8 @@ class TestBlockMeta(IntegrationTestCase):
self.assertEqual(139 << 20, _get_filesystem_size(dev, p2))
self.assertEqual(50 << 20, _get_filesystem_size(dev, p3))
+ @skipIf(distro.lsb_release()['release'] < '20.04',
+ 'old lsblk will not list info about extended partitions')
def test_mix_of_operations_msdos(self):
# a test that keeps, creates, resizes, and deletes a partition
# including handling of extended/logical
diff --git a/tests/unittests/test_compat.py b/tests/unittests/test_compat.py
new file mode 100644
index 0000000..0ba5002
--- /dev/null
+++ b/tests/unittests/test_compat.py
@@ -0,0 +1,34 @@
+# This file is part of curtin. See LICENSE file for copyright and license info.
+
+from unittest import mock
+
+from curtin import compat
+from .helpers import CiTestCase
+
+
+class TestUtilLinuxVer(CiTestCase):
+ @mock.patch('curtin.util.subp')
+ def test_ul_ver(self, m_subp):
+ m_subp.return_value = ('losetup from util-linux 2.31.1', '')
+ self.assertEqual('2.31.1', compat._get_util_linux_ver())
+
+ @mock.patch('curtin.util.subp')
+ def test_ul_malformed(self, m_subp):
+ m_subp.return_value = ('losetup from util-linux asdf', '')
+ self.assertEqual(None, compat._get_util_linux_ver())
+
+ @mock.patch('curtin.compat._get_util_linux_ver')
+ def test_verpass(self, m_gulv):
+ m_gulv.return_value = '1.23.4'
+ self.assertTrue(compat._check_util_linux_ver('1.20'))
+
+ @mock.patch('curtin.compat._get_util_linux_ver')
+ def test_verfail(self, m_gulv):
+ m_gulv.return_value = '1.23.4'
+ self.assertFalse(compat._check_util_linux_ver('1.24'))
+
+ @mock.patch('curtin.compat._get_util_linux_ver')
+ def test_verfatal(self, m_gulv):
+ m_gulv.return_value = '1.23.4'
+ with self.assertRaisesRegex(RuntimeError, '.*my feature.*'):
+ compat._check_util_linux_ver('1.24', 'my feature', fatal=True)
Follow ups
