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

Few small fixes #376

Merged

Conversation

vojtechtrefny
Copy link
Member

Fix for three bugs from freedesktop bugzilla.

Copy link
Contributor

@vpodzime vpodzime left a comment

Choose a reason for hiding this comment

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

Looks good to me otherwise.

@@ -126,6 +126,11 @@ ENV{ID_PART_ENTRY_SCHEME}=="gpt", \
ENV{ID_PART_ENTRY_TYPE}=="c12a7328-f81f-11d2-ba4b-00a0c93ec93b|21686148-6449-6e6f-744e-656564454649|a19d880f-05fc-4d3b-a006-743f0f84911e|e6d6d379-f507-44c2-a23c-238f2a3df928|e3c9e316-0b5c-4db8-817d-f92df00215ae|de94bba4-06d1-4d40-a16a-bfd50179d6ac", \
ENV{UDISKS_IGNORE}="1"

# ZFS member partitions
ENV{ID_PART_ENTRY_SCHEME}=="gpt", \
Copy link
Contributor

Choose a reason for hiding this comment

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

Really only on GPT?

Copy link
Member Author

Choose a reason for hiding this comment

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

I have no idea. This is what ZFSOnLinux FAQ says to do -- https://github.com/zfsonlinux/zfs/wiki/FAQ#udisks2-creating-devmapper-entries-for-zvol

Copy link
Contributor

Choose a reason for hiding this comment

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

OK

Copy link

@Trucido Trucido Aug 14, 2017

Choose a reason for hiding this comment

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

original reporter of the udisks rule for zfs_member here (disclaimer: not officially affiliated with ZoL); let me elaborate if there is any confusion:

ZFS on MBR partitions/slices is extremely rare, this was to avoid unnecessary scanning.

Edit: Also, I think maybe the partition type codes in the rule are only in GPT anyhow, but it is always labeled zfs_member from what I can tell.

In WHOLE_DISK mode (just giving ZFS entire disks to do it's thing, the recommended method) ZoL creates two GPT partitions (mainly just to identify itself, not sure if this is the case on solaris).
If user chooses partitions/slices instead of giving entire disks to ZFS; all of the guides describe creating GPT - with the same part ID's (and BSD creates GPT slices by default with same part ID's IIRC)

so it's safe to say yes GPT, besides rare setups (worst case scenario for the rare MBR setups would be they see some zfs devices in udisks2 which is harmless; they can't be mounted ejected or anything). so let's reduce MBR scanning overhead for these extremely rare ZFS installations.

we're still missing one more ID for dedicated log disk or partition (ZIL/SLOG), but this is only an issue if a dedicated logs device was added to array (less common, rare for desktops).
I haven't had time to track down the ZIL/SLOG partition ID and verify it's always the same ID (etc. etc.).

The only exception to "almost-always-gpt" thing MIGHT be the above mentioned missing ID for logs disk/partition (because it could potentially be on a MBR disk shared with other partitions), but is still likely to be GPT.

To reduce confusion, dedicated log or 'logs' is also called ZIL or SLOG. think of it as an external dedicated journal device attached to a larger ZFS array - only utilized for small O_SYNC writes. desktop users are unlikely to have arrays which would benefit enough from it IMO which is why I said uncommon and rare for desktops

Also, if someone could tell me where the proper place is to report additions to this rule (after I find the above mentioned log partition ID) I'd appreciate it since I originally reported this on freedesktop.org's bugzilla

Please excuse any typos; briefly written after waking up without coffee.

Copy link
Contributor

Choose a reason for hiding this comment

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

ZFS on MBR partitions/slices is extremely rare, this was to avoid unnecessary scanning.

This is not about unnecessary scanning. The udev rule only compares a few properties that are already set anyway. So the real question is if the above combination is the only one it should match. And it looks like it isn't. What if somebody for example has an MBR-partitioned SSD and wants to use one of the partitions as a cache for ZFS?

@vpodzime
Copy link
Contributor

vpodzime commented Aug 15, 2017

I originally reported this on freedesktop.org's bugzilla

Please file issues here at GitHub.

@vpodzime vpodzime closed this Aug 15, 2017
@vpodzime vpodzime reopened this Aug 15, 2017
@vojtechtrefny vojtechtrefny merged commit 8537964 into storaged-project:master Aug 22, 2017
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