Move comment and compatibility properties to the props object.

[colmbuckley]

Motivation and Context

As discussed in #12261, the comment and compatibility properties are stored in the pool config object rather than the properties object. This PR changes the logic to store and load them from the DMU_OT_POOL_PROPS object.

It would also be desirable to detect the legacy configuration and allow migration to the new location [WIP].

Description

In spa_ld_get_props() we load the string values from the props ZAP into the spa_comment and spa_compatibility variables. A couple of helper functions, spa_prop_fstr() and spa_stralloc() have been written to facilitate this.

In spa_ld_parse_config() we no longer look for the ZPOOL_PROP_COMMENT and ZPOOL_PROP_COMPATIBILITY config entries.

In spa_sync_props() we move the handling of these properties from the config section to the props section.

In order to migrate from the old format to the new, there are a few possibilities:

• Silently (or with a warning) delete the entries from the config and add them to props.
  • My concern is that this would lead to this pool then being incorrectly imported on older versions (these properties would be lost, or discarded on import)
• Allow both to exist indefinitely, preferring the props version where both are present.
  • Danger: old software versions would see the config version while new would see the props version, which might be a problem if they differ.
• Output a warning, allowing the user to take action to remediate the issue.

Honestly, none of these are great options. The third is probably the safest as well as the easiest to implement.

I'm on the fence about this: on the one hand, it would be cleanest to actually have all the configurable pool properties stored in the props object, and there would be fewer special cases to deal with. On the other hand (especially given that both of these properties are just hints to userland rather than influencing kernel behavior), there's no real harm in having them in the config object, and a possible small benefit in having them visible in the cachefile before import.

Thoughts?

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Performance enhancement (non-breaking change which improves efficiency)
• Code cleanup (non-breaking change which makes code smaller or more readable)
• Breaking change (fix or feature that would cause existing functionality to change)
• Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
• Documentation (a change to man pages or other documentation)

Checklist:

• My code follows the OpenZFS code style requirements.
• I have updated the documentation accordingly.
• I have read the contributing document.
• I have added tests to cover my changes.
• I have run the ZFS Test Suite with this change applied.
• All commit messages are properly formatted and contain Signed-off-by.

[colmbuckley]

@behlendorf PTAL. I'm leaning towards just not bothering with this, to be honest. The props and config objects are handled differently enough on import and sync that actually implementing this migration might not be nice and clean.

What do you think?

[behlendorf]

@colmbuckley after briefly starting down this same path, I ended up feeling the same way. The main problem as I see it is that making this change really wouldn't simplify things the way we want it to. Yes, everything would be in one object (which is nice) but we'd also need to lug around some ugly migration code potentially forever (which is not nice). Given that, and all the non-ideal migration options you mentioned, I'm also inclined to leave these values where they are and not bother with this.