saltstack / saltstack/salt

[BUG] parted_partitions set id is not possible due to sfdisk type check

Open
#62,804 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug needs-triage
Dominant language
Python
Stars
15.7k
Forks
5.6k
Avg merge
2d 44m
Merged PRs (30d)
80

Description

Description
I ran into an issue with parted_partitions.set_id.
it’s using sfdisk to set the id, and checks if the partition type matches the list available in sfdisk -T,
which returns ID name pairs like

sfdisk -T
Id  Name

 0  Empty
 1  FAT12
 2  XENIX root
...

but sfdisk --change-id only accepts uuid types when using opt
( deprecation note: this should also be --part-type not --change-id on newer versions of sfdisk )

for example:

sfdisk  sfdisk --change-id /dev/sda 2 83

fails with partition 2: failed to set partition type

but the same "type" via uuid as listed in fdisk will change the id correctly.

sfdisk --part-type /dev/nvme0n1 2 0FC63DAF-8483-4772-8E79-3D69D8477DE4

since parted_partitions.set_id only allows the ## IDs, we can’t pass a UUID, but the sfdisk call inside fails if given a ## ID.

Setup
running on an i4i.4xlarge in AWS with SLES 15 SP4.
A free partition is available, in my case a 100GB unformatted primary partition at /dev/nvme0n1p2

Steps to Reproduce the behavior
verify partition present

fdisk -l
Disk /dev/nvme0n1: 150 GiB, 161061273600 bytes, 314572800 sectors
Disk model: Amazon Elastic Block Store
Units: sectors of 1 * 512 = 512 bytes
Sector size (logical/physical): 512 bytes / 4096 bytes
I/O size (minimum/optimal): 4096 bytes / 4096 bytes
Disklabel type: gpt
Disk identifier: 8F39ABE5-CDCC-4432-863E-9AA32257B181

Device             Start       End   Sectors  Size Type
/dev/nvme0n1p1      2048     18431     16384    8M BIOS boot
/dev/nvme0n1p2     18432 209733631 209715200  100G Linux filesystem
/dev/nvme0n1p3 209733632 314572766 104839135   50G Linux filesystem

run partition.system_types to see available types

salt-call --local partition.system_types

local:
    ----------
    0:
        Empty
    1:
        FAT12
    10:
        OPUS
    11:
        Hidden
    12:
        Compaq
    14:
        Hidden
    16:
        Hidden
    17:
        Hidden
    18:
        AST
    1b:
        Hidden
    1c:
        Hidden
    1e:
        Hidden
    2:
        XENIX
    24:
        NEC
    27:
        Hidden
    3:
        XENIX
    39:
        Plan
    3c:
        PartitionMagic
    4:
        FAT16
    40:
        Venix
    41:
        PPC
    42:
        SFS
    4d:
        QNX4.x
    4e:
        QNX4.x
    4f:
        QNX4.x
    5:
        Extended
    50:
        OnTrack
    51:
        OnTrack
    52:
        CP/M
    53:
        OnTrack
    54:
        OnTrackDM6
    55:
        EZ-Drive
    56:
        Golden
    5c:
        Priam
    6:
        FAT16
    61:
        SpeedStor
    63:
        GNU
    64:
        Novell
    65:
        Novell
    7:
        HPFS/NTFS/exFAT
    70:
        DiskSecure
    75:
        PC/IX
    8:
        AIX
    80:
        Old
    81:
        Minix
    82:
        Linux
    83:
        Linux
    84:
        OS/2
    85:
        Linux
    86:
        NTFS
    87:
        NTFS
    88:
        Linux
    8e:
        Linux
    9:
        AIX
    93:
        Amoeba
    94:
        Amoeba
    9f:
        BSD/OS
    a:
        OS/2
    a0:
        IBM
    a5:
        FreeBSD
    a6:
        OpenBSD
    a7:
        NeXTSTEP
    a8:
        Darwin
    a9:
        NetBSD
    ab:
        Darwin
    af:
        HFS
    b:
        W95
    b7:
        BSDI
    b8:
        BSDI
    bb:
        Boot
    bc:
        Acronis
    be:
        Solaris
    bf:
        Solaris
    c:
        W95
    c1:
        DRDOS/sec
    c4:
        DRDOS/sec
    c6:
        DRDOS/sec
    c7:
        Syrinx
    da:
        Non-FS
    db:
        CP/M
    de:
        Dell
    df:
        BootIt
    e:
        W95
    e1:
        DOS
    e3:
        DOS
    e4:
        SpeedStor
    ea:
        Linux
    eb:
        BeOS
    ee:
        GPT
    ef:
        EFI
    f:
        W95
    f0:
        Linux/PA-RISC
    f1:
        SpeedStor
    f2:
        DOS
    f4:
        SpeedStor
    fb:
        VMware
    fc:
        VMware
    fd:
        Linux
    fe:
        LANstep
    ff:
        BBT

attempt to change the partition type:

salt-call --local partition.set_id /dev/nvme0n1 2 29
[ERROR   ] Command 'sfdisk' failed with return code: 1
[ERROR   ] stdout: sfdisk: change-id is deprecated in favour of --part-type
sfdisk: /dev/nvme0n1: partition 2: failed to set partition type
[ERROR   ] retcode: 1
[ERROR   ] Command 'sfdisk' failed with return code: 1
[ERROR   ] output: sfdisk: change-id is deprecated in favour of --part-type
sfdisk: /dev/nvme0n1: partition 2: failed to set partition type
local:
    - sfdisk: change-id is deprecated in favour of --part-type
    - sfdisk: /dev/nvme0n1: partition 2: failed to set partition type

failure to set as the underlying sfdisk command does not accept "29" as a valid uuid
failure also occurs if using uuid

salt-call --local partition.set_id /dev/nvme0n1 2 A19D880F-05FC-4D3B-A006-743F0F84911E
Error running 'partition.set_id': Invalid system_id passed to partition.set_id

because it does not match the output of system_types
https://github.com/saltstack/salt/blob/master/salt/modules/parted_partition.py#L376
which uses compares the input type id to the output of sfdisk -T
https://github.com/saltstack/salt/blob/master/salt/modules/parted_partition.py#L395
but omits the -X label portion to differentiate between mbr and gpt ids.

the call to sfdisk -T needs to be aware if the partition latyout if mbr and gpt and set the '-X label' to match

Expected behavior
salt-call --local partition.set_id /dev/DEVICE PARTITION UUID
should work on gpt partitions

salt-call --local partition.set_id /dev/DEVICE PARTITION ID
should work on MBR partitions

error message should be clear if using UUID on MBR or ID on GPT

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in salt/modules/parted_partition.py at the system_types and partition.set_id logic, then reproduce the reported sfdisk -T and partition.set_id commands on MBR and GPT layouts. Done means GPT accepts UUIDs, MBR accepts numeric IDs, and invalid label/type combinations produce clear errors.

Written by the indexing model from the issue text.

Assessment

Tech stack
linux, python
Domain
infrastructure, operating-systems
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.