Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add partition labels support #30

Closed
wants to merge 1 commit into from
Closed

Add partition labels support #30

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ let
env = helper.device-id q.device;
in
''
parted -s "''${${env}}" mkpart ${x.part-type} ${x.fs-type or ""} ${x.start} ${x.end}
parted -s "''${${env}}" mkpart ${x.part-type} ${x.part-label} ${x.fs-type or ""} ${x.start} ${x.end}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure this is correct?

  mkpart PART-TYPE [FS-TYPE] START END     make a partition

I don't see any label support in parted for this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://www.gnu.org/software/parted/manual/html_chapter/parted_2.html#SEC18

Copy link
Contributor Author

@Baitinq Baitinq Aug 26, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Mic92 Im seeing conflicting info. On the arch wiki for parted (https://wiki.archlinux.org/title/Parted) it says:

(parted) mkpart part-type-or-part-label fs-type start end

    part-type-or-part-label is interpreted differently based on the partition table:
        MBR: the parameter is interpreted as part-type, which can be one of primary, extended or logical.
        GPT: the parameter is interpreted as part-label, which sets the [PARTLABEL](https://wiki.archlinux.org/title/PARTLABEL) attribute of the partition. The partition label always has to be set, since mkpart does not allow to create partitions with empty label.

EDIT: With this I understand we can only set labels using mkpart if using GPT (by using the method in this pr but not setting any fs-type, effectively replacing fs-type by fs-label)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. In this case I would pass down the format type in create.table down to create.partition:

    ${concatStrings (imap (index: create-f (q // { inherit index format; })) x.partitions)}

And than only pass either x.part-type or x.part-label to parted based on the format type (gpt or mbr).
Otherwise disko might pass too many arguments to mkpart

Copy link
Contributor

@yajo yajo Aug 29, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The man page is interesting:

          mkpart [part-type name fs-type] start end
                 Create a new partition. part-type may be specified
                 only with msdos and dvh partition tables, it should
                 be one of "primary", "logical", or "extended".
                 name is required for GPT partition tables and fs-
                 type is optional.  fs-type can be one of "btrfs",
                 "ext2", "ext3", "ext4", "fat16", "fat32", "hfs",
                 "hfs+", "linux-swap", "ntfs", "reiserfs", "udf", or
                 "xfs".

          name partition name
                 Set the name of partition to name. This option
                 works only on Mac, PC98, and GPT disklabels. The
                 name can be placed in double quotes, if necessary.
                 And depending on the shell may need to also be
                 wrapped in single quotes so that the shell doesn't
                 strip off the double quotes.

It seems to indicate the same as https://github.com/nix-community/disko/pull/30/files#r956295248.

Maybe a good option is to assert that you're getting either a part-type for msdos and dvh tables, or a part-label for gpt tables. Or to use parted name to be specific about what it's doing.

# ensure /dev/disk/by-path/..-partN exists before continuing
udevadm trigger --subsystem-match=block; udevadm settle
${optionalString (x.bootable or false) ''
Expand Down