Re: [PATCH] system-connections: restore previous connection settings if update failed by plugin

[Andrey Borzenkov <arvidjaar gmail com>]

Ping.  Unfortunately, the problem is real - due to the way system
connections are stored here in Mandriva it is too hard to implement
full editing, so I need to fail changing connection sometimes.

I would be happy to work on this patch so it is acceptable upstream. Thank  you!

On Wed, Nov 24, 2010 at 9:44 PM, Andrey Borzenkov <arvidjaar gmail com> wrote:
> Currently NM first updates in-memory connection and then requests
> plugin to store definition. If plugin failed to store for whatever
> reason we are left with new memory state and stale permanent copy.
> What makes is worse, nm-connection-editor does not indicate any
> error in this case, so user is unaware that all settings are lost
> after NM is restarted.
>
> Save connection copy before calling plugin and revert to it
> if plugin indicated failure.
>
> Signed-off-by: Andrey Borzenkov <arvidjaar gmail com>
>
> ---
>  src/system-settings/nm-sysconfig-connection.c |   15 +++++++++++++--
>  1 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/src/system-settings/nm-sysconfig-connection.c b/src/system-settings/nm-sysconfig-connection.c
> index 73906d2..5159407 100644
> --- a/src/system-settings/nm-sysconfig-connection.c
> +++ b/src/system-settings/nm-sysconfig-connection.c
> @@ -245,6 +245,9 @@ typedef struct {
>        /* Update */
>        NMConnection *connection;
>
> +       /* Rollback */
> +       NMConnection *old_connection;
> +
>        /* Secrets */
>        char *setting_name;
>        char **hints;
> @@ -270,6 +273,7 @@ polkit_call_new (NMSysconfigConnection *self,
>        call->context = context;
>        call->cancellable = g_cancellable_new ();
>        call->connection = connection;
> +       call->old_connection = NULL;
>        call->setting_name = g_strdup (setting_name);
>        if (hints)
>                call->hints = g_strdupv ((char **) hints);
> @@ -287,6 +291,8 @@ polkit_call_free (PolkitCall *call)
>  {
>        if (call->connection)
>                g_object_unref (call->connection);
> +       if (call->old_connection)
> +               g_object_unref (call->old_connection);
>        g_free (call->setting_name);
>        if (call->hints)
>                g_strfreev (call->hints);
> @@ -303,9 +309,11 @@ con_update_cb (NMSettingsConnectionInterface *connection,
>  {
>        PolkitCall *call = user_data;
>
> -       if (error)
> +       if (error) {
> +
> +               nm_sysconfig_connection_update (call->self, call->old_connection, FALSE, NULL);
>                dbus_g_method_return_error (call->context, error);
> -       else
> +       } else
>                dbus_g_method_return (call->context);
>
>        polkit_call_free (call);
> @@ -357,6 +365,9 @@ pk_update_cb (GObject *object, GAsyncResult *result, gpointer user_data)
>                goto out;
>        }
>
> +       /* Save old connection so we can restore it later */
> +       call->old_connection = nm_connection_duplicate (NM_CONNECTION (self));
> +
>        /* Update our settings internally so the update() call will save the new
>         * ones.  We don't let nm_sysconfig_connection_update() handle the update
>         * signal since we need our own callback after the update is done.
> --
> tg: (f6f8ef2..) u/conn-update (depends on: upstream/master)
>