Skip to content

pkg/disk: add XFS agcount option in disk.yaml#2496

Merged
achilleas-k merged 2 commits into
osbuild:mainfrom
jbtrystram:disk_yaml_custom_xfs_agcount
Jul 9, 2026
Merged

pkg/disk: add XFS agcount option in disk.yaml#2496
achilleas-k merged 2 commits into
osbuild:mainfrom
jbtrystram:disk_yaml_custom_xfs_agcount

Conversation

@jbtrystram

Copy link
Copy Markdown
Contributor

The mkfs.xfs stage already support setting the XFS agcount so this is wiring it up to a disk.yaml option.

I went with a similar approach as #1825

Assisted-by: Opencode.ai <Opus-4.6>

@achilleas-k achilleas-k left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the PR. Small change request for the configuration structure for consistency.

Comment thread pkg/disk/filesystem.go Outdated
type MkfsOptions struct {
Verity bool `json:"verity,omitempty" yaml:"verity,omitempty"`
Geometry *MkfsOptionGeometry `json:"geometry,omitempty" yaml:"geometry,omitempty"`
XFS *MkfsOptionXFS `json:"xfs,omitempty" yaml:"xfs,omitempty"`

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't like the inconsistency here of having an fstype-specific option block only for this option. The geometry and verity options are for fat and ext4 respectively and don't define a sub-struct for the filesystems. Let's put the AGCount under MkfsOptions directly for consistency.
We can rethink the structure and whether we want to make sub-structs for each filesystem another time.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done ! thanks for the review

@jbtrystram

Copy link
Copy Markdown
Contributor Author

Tested this with a fresh build of FCOS to confirm everything works :

core@cosa-devsh:~$ cat /usr/lib/image-builder/bootc/disk.yaml 
# We don't want osbuild to inject mount units for the
# disks as we will change the disks uuids anyway
mount_configuration: none
partition_table:
  type: "gpt"
  uuid: "00000000-0000-4000-a000-000000000001"
  # Reserve the last 2048 512-byte blocks for CoreOS which moves the
  # secondary GPT header to the end of the disk on first boot.
  # See https://github.com/coreos/fedora-coreos-config/blob/7fbf510367d495529d2a915707b547d4c663f609/overlay.d/05core/usr/lib/dracut/modules.d/35coreos-ignition/coreos-gpt-setup.sh#L30
  extra_padding: 1031168
  partitions:
    # BIOS Boot partition — required for legacy BIOS boot on x86_64
    - label: "BIOS-BOOT"
      size: "1 MiB"
      bootable: true
      type: 21686148-6449-6E6F-744E-656564454649

    # EFI System Partition
    - label: "EFI-SYSTEM"
      size: "127 MiB"
      type: C12A7328-F81F-11D2-BA4B-00A0C93EC93B
      payload_type: "filesystem"
      payload:
        type: "vfat"
        mountpoint: "/boot/efi"
        label: "EFI-SYSTEM"
        fstab_options: "umask=0077,shortname=winnt"
        fstab_freq: 0
        fstab_passno: 2

    # Extended Boot Loader partition
    - label: "boot"
      size: "384 MiB"
      type: BC13C2FF-59E6-4262-A352-B275FD6F7172
      payload_type: "filesystem"
      payload:
        type: "ext4"
        uuid: "96d15588-3596-4b3c-adca-a2ff7279ea63"
        mountpoint: "/boot"
        label: "boot"
        fstab_options: "defaults"
        fstab_freq: 0
        fstab_passno: 0

    # Root partition — DPS type for x86_64
    - label: "root"
      type: 4F68BCE3-E8CD-4DB1-96E7-FBCAF984B709
      payload_type: "filesystem"
      payload:
        type: "xfs"
        uuid: "910678ff-f77e-4a7d-8d53-86f2ac47a823"
        mountpoint: "/"
        label: "root"
        fstab_options: "defaults"
        fstab_freq: 0
        fstab_passno: 0
        mkfs_options:
          xfs:
            agcount: 2
core@cosa-devsh:~$ sudo xfs_info /sysroot
meta-data=/dev/vda4              isize=512    agcount=2, agsize=1244928 blks
         =                       sectsz=512   attr=2, projid32bit=1
         =                       crc=1        finobt=1, sparse=1, rmapbt=1
         =                       reflink=1    bigtime=1 inobtcount=1 nrext64=1
         =                       exchange=1   metadir=0
data     =                       bsize=4096   blocks=2489856, imaxpct=25
         =                       sunit=0      swidth=0 blks
naming   =version 2              bsize=4096   ascii-ci=0, ftype=1, parent=1
log      =internal log           bsize=4096   blocks=16384, version=2
         =                       sectsz=512   sunit=0 blks, lazy-count=1
realtime =none                   extsz=4096   blocks=0, rtextents=0
         =                       rgcount=0    rgsize=0 extents
         =                       zoned=0      start=0 reserved=0

The mkfs.xfs stage already support setting the XFS `agcount` so this is
wiring it up to a `disk.yaml` option.

I went with a similar approach as osbuild#1825

Assisted-by: Opencode.ai <Opus-4.6>
@jbtrystram jbtrystram force-pushed the disk_yaml_custom_xfs_agcount branch from e2f3741 to a405699 Compare July 8, 2026 15:36

@achilleas-k achilleas-k left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice work. Thank you!

@achilleas-k achilleas-k enabled auto-merge July 9, 2026 11:25
@achilleas-k achilleas-k added this pull request to the merge queue Jul 9, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 9, 2026
@achilleas-k

Copy link
Copy Markdown
Member

Adding to the merge queue again. The failure should be fixed by #2467 (which is already on the queue right now).

@achilleas-k achilleas-k added this pull request to the merge queue Jul 9, 2026
Merged via the queue into osbuild:main with commit c04ea67 Jul 9, 2026
50 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants