One of the next things on my agenda is to do mass, quick-and-dirty deprecations -- pcmk__*() with public wrapper, no best practices or refactors otherwise, to ensure we get as much deprecated as possible in time.

So this works out.

enum cluster_type_e: deprecate this name and create an equivalent enum pcmk_cluster_type (the enum value names are fine and do not need to be deprecated)

This won't work as written. We can't typedef one enum to another (I found out). We also can't reuse value names in both enums, because it causes a redefinition error. Also, if we create new values for enum cluster_type_e and use the current names in enum pcmk_cluster_type, we get type-checker errors:

enum pcmk_cluster_type {
    /* These aren't bit flags, so consecutive integers would be fine. The hex
     * values are for backward compatibility.
     */
    pcmk_cluster_unknown    = 0x0001,   //!< Unknown cluster layer
    pcmk_cluster_invalid    = 0x0002,   //!< Invalid cluster layer
    pcmk_cluster_corosync   = 0x0020,   //!< Corosync Cluster Engine
};

enum cluster_type_e {
    cluster_type_unknown    = pcmk_cluster_unknown,
    cluster_type_invalid    = pcmk_cluster_invalid,
    cluster_type_corosync   = pcmk_cluster_corosync,
};

cluster.c: In function 'get_cluster_type':
cluster.c:330:22: error: comparison between 'enum cluster_type_e' and 'enum pcmk_cluster_type' [-Werror=enum-compare]
  330 |     if (cluster_type != pcmk_cluster_unknown) {
... etc. ...

These enum-compare errors would go away internally after I replaced internal uses of enum cluster_type_e with enum pcmk_cluster_type, but they would appear in external legacy code that uses enum cluster_type_e.

We could just accept that limitation. External code could compile without -Wenum-compare enabled. Any other approach I can think of (like typedef-ing an enum to pcmk_cluster_type_t) would complicate doxygen and require an enum somewhere else...

On that note, considering that Corosync has been the only supported cluster type for a long time now, I wonder to what extent this stuff even needs to be public. We can always add public API functions again after branching to 3.0.0. Might even make some naming issues easier in the interim, if we can work with things internally without introducing new public names yet.

Edit: Nevermind, sbd uses it :(

deprecate (via doxygen comments)

Why no compat header?

In T788#12084, @nrwahl2 wrote:

if we create new values for enum cluster_type_e and use the current names in enum pcmk_cluster_type, we get type-checker errors:

Ah right, whenever we did this before, we used new value names as well. Let's define new names like pcmk_cluster_type_corosync. (Or maybe pcmk_cluster_layer/pcmk_cluster_layer_corosync.)

In T788#12086, @nrwahl2 wrote:

deprecate (via doxygen comments)

Why no compat header?

Because they're still needed internally -- not worth wrappers when we're about to bring them internal

In T788#12087, @kgaillot wrote:

Ah right, whenever we did this before, we used new value names as well. Let's define new names like pcmk_cluster_type_corosync. (Or maybe pcmk_cluster_layer/pcmk_cluster_layer_corosync.)

Sounds good. We're still not going to be able to alias enum cluster_type_e to enum pcmk_cluster_layer though. I think that'll be fine... existing functions like get_cluster_type() can continue enum cluster_type_e, while their new versions can return enum pcmk_cluster_layer. Shouldn't disrupt existing code, and anything that uses one new piece can use all new pieces and/or cast between them explicitly.

(Edited after realizing an oversight)

For 3.0.0 (T793)... For pcmk_cluster_t, we can replace all deprecated struct members except destroy() and cpg with a void *private member that can be used internally for a new internal struct containing the deprecated members.

Why can't destroy and cpg become part of the void *private member at 3.0.0? It seems that's the point of creating the setter functions for those, so that external users no longer need direct access.

In T788#12132, @nrwahl2 wrote:

For 3.0.0 (T793)... For pcmk_cluster_t, we can replace all deprecated struct members except destroy() and cpg with a void *private member that can be used internally for a new internal struct containing the deprecated members.

Why can't destroy and cpg become part of the void *private member at 3.0.0? It seems that's the point of creating the setter functions for those, so that external users no longer need direct access.

The goal is to preserve the ability of existing versions of sbd to build with newer Pacemaker. So, only members that sbd doesn't use can move into private. The timeline is: update sbd to use the new accessors -> that makes it into an sbd release -> long time -> Pacemaker no longer supports distro versions with the older sbd versions -> move the remaining members into private.