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.

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

s390x: create 'by-partlabel' symlinks for labeled filesystems on DASD-CDL partitions #2473

Closed

Conversation

nikita-dubrovskii
Copy link
Contributor

CDL formatted DASD disks support only up to 3 partitions and don't use GPT, so there is no /dev/disk/by-partlabel/* symlinks but only /dev/disk/by-label/*.

…-CDL partitions

CDL formatted DASD disks support only up to 3 partitions and don't use GPT,
so there is no `/dev/disk/by-partlabel/*` symlinks but only `/dev/disk/by-label/*`.
@@ -0,0 +1,15 @@
# CoreOS-specific symlinks for DASD disks
Copy link
Member

Choose a reason for hiding this comment

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

Looks sane to me but...this isn't actually CoreOS specific, is it? I think we could upstream this to systemd, right?

Copy link
Contributor Author

@nikita-dubrovskii nikita-dubrovskii Jun 21, 2023

Choose a reason for hiding this comment

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

TBH i don't know. There is an issue when installing CoreOS on zVM with config generated by Butane + boot-device syntax sugar, where by-partlabel is used by default for LUKS encryption.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

/cc @bgilbert

@nikita-dubrovskii
Copy link
Contributor Author

coreos/butane#453

@bgilbert
Copy link
Contributor

We shouldn't do this. Those partitions don't actually have partition labels, but this PR is incorrectly claiming that they do, and repurposing a completely different label (the filesystem label) just to have a name to use for the symlink. This seems likely to increase confusion without solving an actual problem. The proper fix for coreos/butane#453 is to add Butane layouts for s390x, not to try to force the x86_64 template to work on s390x.

I'll go ahead and close.

@bgilbert bgilbert closed this Jun 21, 2023
@cgwalters
Copy link
Member

OK, I do generally agree, however I would say that this is pretty widespread issue for us, beyond just that LUKS example. If I understand correctly, every use of /dev/disk/by-partlabel we recommend in docs is broken on s390x. That's...a lot! It's used in e.g. https://docs.fedoraproject.org/en-US/fedora-coreos/storage/ too.

@bgilbert
Copy link
Contributor

Yes, it's always been like that. If we want to switch our docs to /dev/disk/by-label, we can likely do that in some places, though probably not when reprovisioning storage where the underlying filesystem is going to be wiped (since that would create symlink races). We might need to have s390x-specific instructions in some places.

@nikita-dubrovskii
Copy link
Contributor Author

We shouldn't do this. Those partitions don't actually have partition labels, but this PR is incorrectly claiming that they do, and repurposing a completely different label (the filesystem label) just to have a name to use for the symlink. This seems likely to increase confusion without solving an actual problem.

I had similar concerns about this udev-rule, on the other hand DASDs don't support GPT and usage of filesystems labels as partlabels shouldn't add any issues.

The proper fix for coreos/butane#453 is to add Butane layouts for s390x, not to try to force the x86_64 template to work on s390x.

Agree. At least i refreshed my memory regarding udev

@nikita-dubrovskii nikita-dubrovskii deleted the dasd-udev branch June 22, 2023 11:08
@bgilbert
Copy link
Contributor

usage of filesystems labels as partlabels shouldn't add any issues.

Well, filesystem and partition labels have different lifecycles. mkfs.ext4 /dev/disk/by-partlabel/foo has no effect on the symlink, while mkfs.ext4 /dev/disk/by-label/foo causes the latter symlink to be deleted.

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.

None yet

3 participants