Skip to content

libnm: Refactor NM_CONNECTION_SERIALIZE_* flags

Andrew Zaborowski requested to merge balrog-kun/NetworkManager:serialize-flags into master

This is a proposal to extend the NM_CONNECTION_SERIALIZE_* flags. Here's the commit message that explains it:

nm-settings-connection.c has the code similar to this in two places:

    /* FIXME: improve NMConnection API so we can avoid the overhead of cloning the connection,
     *   in particular if there are no secrets to begin with. */

    connection_cloned = nm_simple_connection_new_clone(new);

    /* Clear out unwanted secrets */
    _nm_connection_clear_secrets_by_secret_flags(connection_cloned,
                                                 NM_SETTING_SECRET_FLAG_NOT_SAVED
                                                     | NM_SETTING_SECRET_FLAG_AGENT_OWNED);

    secrets = nm_g_variant_ref_sink(
        nm_connection_to_dbus(connection_cloned, NM_CONNECTION_SERIALIZE_ONLY_SECRETS));

It seems the secrets filtering can be done by nm_connection_to_dbus() if the NM_CONNECTION_SERIALIZE_* flags are extended. The current set of flags contains flags that start with NO, ONLY and WITH prefixes, which makes it useless for combining the flags because most combinations of more than one flag don't have a clear interpretation. So they're mostly useful when used alone, i.e. you'll need to add a new enum value for each new subset of settings to be serialized.

To get the most flexibility from a small subset of flags they should either all be of the WITH_* type or NO_* type. In the former case they could be combined to extend the subset of properties serialized, in the latter case each flag would reduce the subset. After trying both options I found it's easier to adapt the current set of flags to the WITH_* schema while keeping binary and source compatibility. This commits changes the set of flags in the following way:

NM_CONNECTION_SERIALIZE_ALL is kept for compatibility but is equivalent to a combination of other flags.

NM_CONNECTION_SERIALIZE_WITH_NON_SECRET is added with the same value as NM_CONNECTION_SERIALIZE_NO_SECRETS, it implies that non-secret properties are included but doesn't prevent including other properties. Since it couldn't be meaningfully combined with any other flag this change shouldn't break compatibility.

Similarly NM_CONNECTION_SERIALIZE_WITH_SECRETS is added with the same value as existing NM_CONNECTION_SERIALIZE_ONLY_SECRETS with the same consideration.

NM_CONNECTION_SERIALIZE_WITH_SECRETS_AGENT_OWNED and the new NM_CONNECTION_SERIALIZE_WITH_SECRETS_SYSTEM_OWNED and NM_CONNECTION_SERIALIZE_WITH_SECRETS_NOT_SAVED add only subsets of secrets and can be combined. For backwards compatibility NM_CONNECTION_SERIALIZE_ONLY_SECRETS is basically ignored when either of these three is present, so that the value: ..ONLY_SECRETS | ..AGENT_OWNED works as previously.

Merge request reports