Review: Approve a lot of groaning but ok.
Diff comments: > diff --git a/curtin/block/__init__.py b/curtin/block/__init__.py > index 2f2b60c..0e13a0d 100644 > --- a/curtin/block/__init__.py > +++ b/curtin/block/__init__.py > @@ -1041,9 +1041,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: Er the way you phrase the comment, it sounds like you should return True here? I can believe that's not the right thing to do but perhaps a little more ranting in comments is appropriate? > + 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 7988f3a..71035f9 100644 > --- a/curtin/commands/block_meta.py > +++ b/curtin/commands/block_meta.py > @@ -854,17 +854,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]): can you just write this as "p_size == 0 and p_start == 0" please? > + # 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) -- https://code.launchpad.net/~dbungert/curtin/+git/curtin/+merge/443380 Your team curtin developers is subscribed to branch curtin:master. -- Mailing list: https://launchpad.net/~curtin-dev Post to : [email protected] Unsubscribe : https://launchpad.net/~curtin-dev More help : https://help.launchpad.net/ListHelp
