curtin-dev team mailing list archive

[Dan Watkins <daniel.watkins@xxxxxxxxxxxxx>]

Diff comments:

> diff --git a/curtin/commands/block_meta.py b/curtin/commands/block_meta.py
> index 1ad49b7..52eae94 100644
> --- a/curtin/commands/block_meta.py
> +++ b/curtin/commands/block_meta.py
> @@ -715,8 +721,28 @@ def verify_exists(devpath):
>          raise RuntimeError("Device %s does not exist" % devpath)
>  
>  
> -def verify_size(devpath, expected_size_bytes):
> -    found_size_bytes = block.read_sys_block_size_bytes(devpath)
> +def _sfdisk_get_partition_info(devpath, sfdisk_info=None):
> +    if not sfdisk_info:
> +        sfdisk_info = block.sfdisk_info(devpath)
> +
> +    entry = [part for part in sfdisk_info['partitions']
> +             if part['node'] == devpath]
> +    if len(entry) != 1:
> +        raise RuntimeError('Device %s not present in sfdisk dump:\n%s' %
> +                           devpath, util.json_dumps(sfdisk_info))
> +    return entry.pop()
> +
> +
> +def verify_size(devpath, expected_size_bytes, sfdisk_info=None):
> +    if not sfdisk_info:
> +        sfdisk_info = block.sfdisk_info(devpath)
> +
> +    part_info = _sfdisk_get_partition_info(devpath, sfdisk_info=sfdisk_info)
> +    # FIXME: stuff this list of msdos extended partition type codes elsewhere

I'd like to see this FIXME addressed before we land.

> +    if part_info.get('type') in ('5', 'f', '85', 'c5'):
> +        found_size_bytes = int(part_info['size']) * 512
> +    else:
> +        found_size_bytes = block.read_sys_block_size_bytes(devpath)
>      msg = (
>          'Verifying %s size, expecting %s bytes, found %s bytes' % (
>           devpath, expected_size_bytes, found_size_bytes))
> @@ -725,19 +751,30 @@ def verify_size(devpath, expected_size_bytes):
>          raise RuntimeError(msg)
>  
>  
> -def verify_ptable_flag(devpath, expected_flag):
> -    if not SGDISK_FLAGS.get(expected_flag):
> +def verify_ptable_flag(devpath, expected_flag, sfdisk_info=None):
> +    if expected_flag not in (list(SGDISK_FLAGS.keys()) +
> +                             list(MSDOS_FLAGS.keys())):
>          raise RuntimeError(
> -            'Cannot verify unknown partition flag: %s', expected_flag)
> +            'Cannot verify unknown partition flag: %s' % expected_flag)
>  
> -    info = block.sfdisk_info(devpath)
> -    if devpath not in info:
> -        raise RuntimeError('Device %s not present in sfdisk dump:\n%s' %
> -                           devpath, util.json_dumps(info))
> +    if not sfdisk_info:
> +        sfdisk_info = block.sfdisk_info(devpath)
> +    if not sfdisk_info:
> +        raise RuntimeError('Failed to extract sfdisk info from %s' % devpath)

Should we be raising this error in all the other places that we're using sfdisk_info?

>  
> -    entry = info[devpath]
> +    entry = _sfdisk_get_partition_info(devpath, sfdisk_info=sfdisk_info)
>      LOG.debug("Device %s ptable entry: %s", devpath, util.json_dumps(entry))
> -    (found_flag, code) = ptable_uuid_to_flag_entry(entry['type'])
> +    found_flag = None
> +    if (sfdisk_info['label'] in ('dos', 'msdos')):
> +        if expected_flag == 'boot':
> +            found_flag = 'boot' if entry.get('bootable') is True else None
> +        elif expected_flag == 'extended':
> +            (found_flag, _code) = ptable_uuid_to_flag_entry(entry['type'])
> +        elif expected_flag == 'logical':
> +            (_parent, partnumber) = block.get_blockdev_for_partition(devpath)
> +            found_flag = 'logical' if int(partnumber) > 4 else None
> +    else:
> +        (found_flag, _code) = ptable_uuid_to_flag_entry(entry['type'])
>      msg = (
>          'Verifying %s partition flag, expecting %s, found %s' % (
>           devpath, expected_flag, found_flag))
> diff --git a/curtin/storage_config.py b/curtin/storage_config.py
> index eccb96b..c0e351d 100644
> --- a/curtin/storage_config.py
> +++ b/curtin/storage_config.py
> @@ -1234,13 +1238,18 @@ def ptable_uuid_to_flag_entry(guid):
>          '9E1A2D38-C612-4316-AA26-8B49521E5A8B': ('prep', '4200'),
>          'A19D880F-05FC-4D3B-A006-743F0F84911E': ('raid', 'fd00'),
>          '0657FD6D-A4AB-43C4-84E5-0933C84B4F4F': ('swap', '8200'),
> -        '0X83': ('linux', '83'),
>          '0XF': ('extended', 'f'),
>          '0X5': ('extended', 'f'),
> +        '0X80': ('boot', '80'),
> +        '0X83': ('linux', '83'),
>          '0X85': ('extended', 'f'),
>          '0XC5': ('extended', 'f'),
>      }
>      name = code = None
> +
> +    # prefix non-uuid guid values with 0x
> +    if guid and '-' not in guid and not guid.upper().startswith('0X'):
> +        guid = '0x' + guid

Why 0x and not 0X?

>      if guid and guid.upper() in guid_map:
>          name, code = guid_map[guid.upper()]
>  
> diff --git a/examples/tests/reuse-msdos-partitions.yaml b/examples/tests/reuse-msdos-partitions.yaml
> new file mode 100644
> index 0000000..bc1dbf1
> --- /dev/null
> +++ b/examples/tests/reuse-msdos-partitions.yaml
> @@ -0,0 +1,135 @@
> +showtrace: true
> +install:
> +   unmount: disabled
> +
> +# The point of this test is to test installing to a disk that contains
> +# a partition that used to be a RAID member where the other parts of
> +# the RAID are not present (the scenario is that the disk was just
> +# grabbed out of a pile of previously used disks and shoved into a
> +# server).
> +
> +# So what it does is to create a RAID0 out of partitions on two disks,
> +# stop the RAID, wipe the superblock on one of them and then install
> +# to the other using a standard partitioning scheme.
> +
> +bucket:
> +  - &setup |
> +    parted /dev/disk/by-id/virtio-disk-a --script -- \
> +        mklabel msdos              \
> +        mkpart primary 1MiB 3073MiB \
> +        mkpart extended 3074MiB 8193MiB \
> +        mkpart logical 3075MiB 5122MiB \
> +        mkpart logical 5123MiB 8192MiB \
> +        set 1 boot on
> +    udevadm settle
> +
> +early_commands:
> +  00-setup-raid: [sh, -exuc, *setup]
> +
> +
> +showtrace: true
> +storage:
> +    version: 1
> +    config:
> +      - id: sda
> +        type: disk
> +        ptable: msdos
> +        model: QEMU HARDDISK
> +        serial: disk-a
> +        name: main_disk
> +        preserve: true
> +        grub_device: true
> +      - id: sdb
> +        type: disk
> +        ptable: msdos
> +        model: QEMU HARDDISK
> +        serial: disk-b
> +        name: extra_disk
> +        wipe: superblock-recursive
> +      - id: sda1
> +        type: partition
> +        number: 1
> +        size: 3072M
> +        device: sda
> +        flag: boot
> +        preserve: true
> +        wipe: superblock
> +      - id: sda2
> +        type: partition
> +        number: 2
> +        size: 5119M
> +        flag: extended
> +        device: sda
> +        preserve: true
> +      - id: sda5
> +        type: partition
> +        number: 5
> +        size: 2047M
> +        flag: logical
> +        device: sda
> +        preserve: true
> +        wipe: superblock
> +      - id: sda6
> +        type: partition
> +        number: 6
> +        size: 3069M
> +        flag: logical
> +        device: sda
> +        preserve: true
> +        wipe: superblock
> +      - id: sdb1
> +        type: partition
> +        number: 1
> +        size: 4GB
> +        device: sdb
> +      - id: volgroup1
> +        name: vg1
> +        type: lvm_volgroup
> +        devices:
> +            - sda5
> +            - sda6
> +      - id: lvmpart1
> +        name: lv1
> +        size: 1G
> +        type: lvm_partition
> +        volgroup: volgroup1
> +      - id: lvmpart2
> +        name: lv2
> +        type: lvm_partition
> +        volgroup: volgroup1
> +      - id: volgroup2
> +        name: ubuntu-vg
> +        type: lvm_volgroup
> +        devices:
> +            - sdb1
> +      - id: ubuntulv1
> +        name: my-storage
> +        size: 1G
> +        type: lvm_partition
> +        volgroup: volgroup2
> +      - id: sda1_root
> +        type: format
> +        fstype: ext4
> +        volume: sda1
> +      - id: lv1_fs
> +        name: storage
> +        type: format
> +        fstype: fat32
> +        volume: lvmpart1
> +      - id: lv2_fs
> +        name: storage
> +        type: format
> +        fstype: ext3
> +        volume: lvmpart2
> +      - id: sda1_mount
> +        type: mount
> +        path: /
> +        device: sda1_root
> +      - id: lv1_mount
> +        type: mount
> +        path: /srv/data
> +        device: lv1_fs
> +      - id: lv2_mount
> +        type: mount
> +        path: /srv/backup
> +        device: lv2_fs

+1 on finding a (more) minimal config that exercises this code.



-- 
https://code.launchpad.net/~raharper/curtin/+git/curtin/+merge/383180
Your team curtin developers is subscribed to branch curtin:master.