On Tue, 2014-09-23 at 16:51 +0200, Christian Hesse wrote:
> By default interface name is 'tun' with an incrementing number (tun0,
> tun1, ...). By specifying 'Interface name' in vpnc config you can change
> the name to something more descriptice.
Mostly looks good. Perhaps one more round, see below...
> ---
> properties/nm-vpnc-dialog.ui | 89
> +++++++++++++++++++++++++++++---------------
> properties/nm-vpnc.c | 54 ++++++++++++++++++++++++++-
> src/nm-vpnc-service.c | 9 ++++-
> 3 files changed, 121 insertions(+), 31 deletions(-)
>
> diff --git a/properties/nm-vpnc-dialog.ui b/properties/nm-vpnc-dialog.ui
> index 168782e..6c46785 100644
> --- a/properties/nm-vpnc-dialog.ui
> +++ b/properties/nm-vpnc-dialog.ui
> @@ -555,7 +555,7 @@ config: DPD idle timeout (our side) 0</property>
> <object class="GtkTable" id="table2">
> <property name="visible">True</property>
> <property name="can_focus">False</property>
> - <property name="n_rows">8</property>
> + <property name="n_rows">9</property>
> <property name="n_columns">3</property>
> <property name="column_spacing">6</property>
> <property name="row_spacing">6</property>
> @@ -574,8 +574,8 @@ config: DPD idle timeout (our side) 0</property>
> <packing>
> <property name="left_attach">2</property>
> <property name="right_attach">3</property>
> - <property name="top_attach">4</property>
> - <property name="bottom_attach">5</property>
> + <property name="top_attach">5</property>
> + <property name="bottom_attach">6</property>
> </packing>
> </child>
> <child>
> @@ -593,8 +593,8 @@ config: DPD idle timeout (our side) 0</property>
> <packing>
> <property name="left_attach">2</property>
> <property name="right_attach">3</property>
> - <property name="top_attach">2</property>
> - <property name="bottom_attach">3</property>
> + <property name="top_attach">3</property>
> + <property name="bottom_attach">4</property>
> </packing>
> </child>
> <child>
> @@ -611,8 +611,8 @@ config: DPD idle timeout (our side) 0</property>
> <packing>
> <property name="left_attach">1</property>
> <property name="right_attach">2</property>
> - <property name="top_attach">5</property>
> - <property name="bottom_attach">6</property>
> + <property name="top_attach">6</property>
> + <property name="bottom_attach">7</property>
> </packing>
> </child>
> <child>
> @@ -626,8 +626,8 @@ config: IPSec secret <group_password></property>
> <packing>
> <property name="left_attach">1</property>
> <property name="right_attach">2</property>
> - <property name="top_attach">4</property>
> - <property name="bottom_attach">5</property>
> + <property name="top_attach">5</property>
> + <property name="bottom_attach">6</property>
> <property name="y_options"></property>
> </packing>
> </child>
> @@ -641,8 +641,8 @@ config: IPSec secret <group_password></property>
> <property
> name="mnemonic_widget">group_password_entry</property>
> </object>
> <packing>
> - <property name="top_attach">4</property>
> - <property name="bottom_attach">5</property>
> + <property name="top_attach">5</property>
> + <property name="bottom_attach">6</property>
> <property name="x_options">GTK_FILL</property>
> <property name="y_options"></property>
> </packing>
> @@ -657,6 +657,8 @@ config: IPSec secret <group_password></property>
> <property name="mnemonic_widget">gateway_entry</property>
> </object>
> <packing>
> + <property name="top_attach">1</property>
> + <property name="bottom_attach">2</property>
> <property name="x_options">GTK_FILL</property>
> <property name="y_options"></property>
> </packing>
> @@ -666,13 +668,27 @@ config: IPSec secret <group_password></property>
> <property name="visible">True</property>
> <property name="can_focus">False</property>
> <property name="xalign">0</property>
> + <property name="label" translatable="yes">_Interface
> name:</property>
> + <property name="use_underline">True</property>
> + <property
> name="mnemonic_widget">interface_name_entry</property>
> + </object>
> + <packing>
> + <property name="x_options">GTK_FILL</property>
> + <property name="y_options"></property>
> + </packing>
> + </child>
> + <child>
> + <object class="GtkLabel" id="label25">
> + <property name="visible">True</property>
> + <property name="can_focus">False</property>
> + <property name="xalign">0</property>
> <property name="label" translatable="yes">G_roup
> name:</property>
> <property name="use_underline">True</property>
> <property name="mnemonic_widget">group_entry</property>
> </object>
> <packing>
> - <property name="top_attach">3</property>
> - <property name="bottom_attach">4</property>
> + <property name="top_attach">4</property>
> + <property name="bottom_attach">5</property>
> <property name="x_options">GTK_FILL</property>
> <property name="y_options"></property>
> </packing>
> @@ -687,6 +703,21 @@ config: IPSec gateway <gateway></property>
> <packing>
> <property name="left_attach">1</property>
> <property name="right_attach">2</property>
> + <property name="top_attach">1</property>
> + <property name="bottom_attach">2</property>
> + <property name="y_options"></property>
> + </packing>
> + </child>
> + <child>
> + <object class="GtkEntry" id="interface_name_entry">
> + <property name="visible">True</property>
> + <property name="can_focus">True</property>
> + <property name="tooltip_text" translatable="yes">Name of
> the tun/tap
> +network interface</property>
> + </object>
> + <packing>
> + <property name="left_attach">1</property>
> + <property name="right_attach">2</property>
> <property name="y_options"></property>
> </packing>
> </child>
> @@ -700,8 +731,8 @@ config: IPSec ID <group_name></property>
> <packing>
> <property name="left_attach">1</property>
> <property name="right_attach">2</property>
> - <property name="top_attach">3</property>
> - <property name="bottom_attach">4</property>
> + <property name="top_attach">4</property>
> + <property name="bottom_attach">5</property>
> <property name="y_options"></property>
> </packing>
> </child>
> @@ -715,8 +746,8 @@ config: IPSec ID <group_name></property>
> <property
> name="mnemonic_widget">user_password_entry</property>
> </object>
> <packing>
> - <property name="top_attach">2</property>
> - <property name="bottom_attach">3</property>
> + <property name="top_attach">3</property>
> + <property name="bottom_attach">4</property>
> <property name="x_options">GTK_FILL</property>
> <property name="y_options"></property>
> </packing>
> @@ -732,8 +763,8 @@ config: Xauth password <password></property>
> <packing>
> <property name="left_attach">1</property>
> <property name="right_attach">2</property>
> - <property name="top_attach">2</property>
> - <property name="bottom_attach">3</property>
> + <property name="top_attach">3</property>
> + <property name="bottom_attach">4</property>
> <property name="y_options"></property>
> </packing>
> </child>
> @@ -749,8 +780,8 @@ config: Xauth username <user_name></property>
> <packing>
> <property name="left_attach">1</property>
> <property name="right_attach">2</property>
> - <property name="top_attach">1</property>
> - <property name="bottom_attach">2</property>
> + <property name="top_attach">2</property>
> + <property name="bottom_attach">3</property>
> <property name="y_options"></property>
> </packing>
> </child>
> @@ -764,8 +795,8 @@ config: Xauth username <user_name></property>
> <property name="mnemonic_widget">user_entry</property>
> </object>
> <packing>
> - <property name="top_attach">1</property>
> - <property name="bottom_attach">2</property>
> + <property name="top_attach">2</property>
> + <property name="bottom_attach">3</property>
> <property name="x_options">GTK_FILL</property>
> <property name="y_options"></property>
> </packing>
> @@ -785,8 +816,8 @@ config: IKE Authmode hybrid</property>
> </object>
> <packing>
> <property name="right_attach">3</property>
> - <property name="top_attach">6</property>
> - <property name="bottom_attach">7</property>
> + <property name="top_attach">7</property>
> + <property name="bottom_attach">8</property>
> </packing>
> </child>
> <child>
> @@ -806,8 +837,8 @@ config: IKE Authmode hybrid</property>
> </child>
> </object>
> <packing>
> - <property name="top_attach">7</property>
> - <property name="bottom_attach">8</property>
> + <property name="top_attach">8</property>
> + <property name="bottom_attach">9</property>
> </packing>
> </child>
> <child>
> @@ -820,8 +851,8 @@ config: CA-File</property>
> <packing>
> <property name="left_attach">1</property>
> <property name="right_attach">2</property>
> - <property name="top_attach">7</property>
> - <property name="bottom_attach">8</property>
> + <property name="top_attach">8</property>
> + <property name="bottom_attach">9</property>
> </packing>
> </child>
> <child>
> diff --git a/properties/nm-vpnc.c b/properties/nm-vpnc.c
> index 7e7dcb3..5d8a682 100644
> --- a/properties/nm-vpnc.c
> +++ b/properties/nm-vpnc.c
> @@ -453,6 +453,7 @@ init_plugin_ui (VpncPluginUiWidget *self,
> GError **error)
> {
> VpncPluginUiWidgetPrivate *priv = VPNC_PLUGIN_UI_WIDGET_GET_PRIVATE
> (self);
> + NMSettingConnection *s_con = NULL;
> NMSettingVPN *s_vpn = NULL;
> GtkWidget *widget;
> GtkListStore *store;
> @@ -466,11 +467,23 @@ init_plugin_ui (VpncPluginUiWidget *self,
> gboolean enabled = FALSE;
> GtkFileFilter *filter;
>
> - if (connection)
> + if (connection) {
> + s_con = nm_connection_get_setting_connection (connection);
> s_vpn = nm_connection_get_setting_vpn (connection);
> + }
>
> priv->group = gtk_size_group_new (GTK_SIZE_GROUP_HORIZONTAL);
>
> + widget = GTK_WIDGET (gtk_builder_get_object (priv->builder,
> "interface_name_entry"));
> + g_return_val_if_fail (widget != NULL, FALSE);
> + gtk_size_group_add_widget (priv->group, GTK_WIDGET (widget));
> + if (s_con) {
> + value = nm_setting_connection_get_interface_name (s_con);
> + if (value && strlen (value))
> + gtk_entry_set_text (GTK_ENTRY (widget), value);
> + }
> + g_signal_connect (G_OBJECT (widget), "changed", G_CALLBACK
> (stuff_changed_cb), self);
> +
> widget = GTK_WIDGET (gtk_builder_get_object (priv->builder,
> "gateway_entry"));
> g_return_val_if_fail (widget != NULL, FALSE);
> gtk_size_group_add_widget (priv->group, GTK_WIDGET (widget));
> @@ -892,6 +905,7 @@ update_connection (NMVpnPluginUiWidgetInterface *iface,
> {
> VpncPluginUiWidget *self = VPNC_PLUGIN_UI_WIDGET (iface);
> VpncPluginUiWidgetPrivate *priv = VPNC_PLUGIN_UI_WIDGET_GET_PRIVATE
> (self);
> + NMSettingConnection *s_con;
> NMSettingVPN *s_vpn;
> GtkWidget *widget;
> char *str;
> @@ -902,9 +916,18 @@ update_connection (NMVpnPluginUiWidgetInterface *iface,
> if (!check_validity (self, error))
> return FALSE;
>
> + s_con = nm_connection_get_setting_connection (connection);
> +
> s_vpn = NM_SETTING_VPN (nm_setting_vpn_new ());
> g_object_set (s_vpn, NM_SETTING_VPN_SERVICE_TYPE, NM_DBUS_SERVICE_VPNC,
> NULL);
>
> + /* Interface name */
> + widget = GTK_WIDGET (gtk_builder_get_object (priv->builder,
> "interface_name_entry"));
> + str = (char *) gtk_entry_get_text (GTK_ENTRY (widget));
> + if (str && strlen (str))
> + g_object_set (G_OBJECT (s_con),
> + NM_SETTING_CONNECTION_INTERFACE_NAME, str,
> NULL);
> +
> /* Gateway */
> widget = GTK_WIDGET (gtk_builder_get_object (priv->builder,
> "gateway_entry"));
> str = (char *) gtk_entry_get_text (GTK_ENTRY (widget));
> @@ -1343,6 +1366,21 @@ import (NMVpnPluginUiInterface *iface, const char
> *path, GError **error)
> s_ip4 = NM_SETTING_IP4_CONFIG (nm_setting_ip4_config_new ());
> nm_connection_add_setting (connection, NM_SETTING (s_ip4));
>
> + /* Interface Name */
> + buf = key_file_get_string_helper (keyfile, "main", "InterfaceName",
> NULL);
> + if (buf) {
> + g_object_set (G_OBJECT (s_con),
> + NM_SETTING_CONNECTION_INTERFACE_NAME, buf,
> NULL);
> + g_free (buf);
> + } else {
> + g_set_error (error,
> + NM_VPNC_IMPORT_EXPORT_ERROR,
> + NM_VPNC_IMPORT_EXPORT_ERROR_NOT_VPNC,
> + "does not look like a %s VPN connection (no
> interface name)",
> + VPNC_PLUGIN_NAME);
> + goto error;
This part will cause most imports to fail, because most connections
don't specify an interface name. I think we should just kill the entire
'else' block here since it's not really a hard error if the name is
missing.
> + }
> +
> /* Gateway */
> buf = key_file_get_string_helper (keyfile, "main", "Host", NULL);
> if (buf) {
> @@ -1582,6 +1620,7 @@ export (NMVpnPluginUiInterface *iface,
> FILE *f;
> const char *value;
> const char *gateway = NULL;
> + const char *interface_name = NULL;
> gboolean enablenat = TRUE;
> gboolean singledes = FALSE;
> const char *groupname = NULL;
> @@ -1614,6 +1653,17 @@ export (NMVpnPluginUiInterface *iface,
> return FALSE;
> }
>
> + value = nm_setting_connection_get_interface_name (s_con);
> + if (value && strlen (value))
> + interface_name = value;
> + else {
> + g_set_error_literal (error,
> + NM_VPNC_IMPORT_EXPORT_ERROR,
> + NM_VPNC_IMPORT_EXPORT_ERROR_BAD_DATA,
> + "connection was incomplete (missing
> interface name)");
> + goto done;
Same kind of thing here; if there is no interface name, it's not a hard
error at all, since vpnc will just pick the first free tun name. SO I
think the entire 'else' block here can die.
> + }
> +
> value = nm_setting_vpn_get_data_item (s_vpn, NM_VPNC_KEY_GATEWAY);
> if (value && strlen (value))
> gateway = value;
> @@ -1729,6 +1779,7 @@ export (NMVpnPluginUiInterface *iface,
> fprintf (f,
> "[main]\n"
> "Description=%s\n"
> + "InterfaceName=%s\n"
We won't always have the interface name though, so I think we have to
conditionally export it like some of the other variables are.
Thoughts?
Dan
> "Host=%s\n"
> "AuthType=1\n"
> "GroupName=%s\n"
> @@ -1770,6 +1821,7 @@ export (NMVpnPluginUiInterface *iface,
> "X-NM-SaveGroupPassword=%s\n"
> "%s\n",
> /* Description */ nm_setting_connection_get_id (s_con),
> + /* InterfaceName */ interface_name,
> /* Host */ gateway,
> /* GroupName */ groupname,
> /* GroupPassword */ group_pw ? group_pw : "",
> diff --git a/src/nm-vpnc-service.c b/src/nm-vpnc-service.c
> index d5bee61..d88a132 100644
> --- a/src/nm-vpnc-service.c
> +++ b/src/nm-vpnc-service.c
> @@ -460,6 +460,7 @@ write_one_property (const char *key, const char *value,
> gpointer user_data)
>
> static gboolean
> nm_vpnc_config_write (gint vpnc_fd,
> + NMSettingConnection *s_con,
> NMSettingVPN *s_vpn,
> GError **error)
> {
> @@ -476,6 +477,8 @@ nm_vpnc_config_write (gint vpnc_fd,
> if (debug)
> write_config_option (vpnc_fd, "Debug 3\n");
>
> + write_config_option (vpnc_fd, "Interface name %s\n",
> nm_setting_connection_get_interface_name(s_con));
> +
> write_config_option (vpnc_fd, "Script " NM_VPNC_HELPER_PATH "\n");
>
> write_config_option (vpnc_fd,
> @@ -553,9 +556,13 @@ real_connect (NMVPNPlugin *plugin,
> {
> NMVPNCPluginPrivate *priv = NM_VPNC_PLUGIN_GET_PRIVATE (plugin);
> NMSettingVPN *s_vpn;
> + NMSettingConnection *s_con;
> gint vpnc_fd = -1;
> gboolean success = FALSE;
>
> + s_con = NM_SETTING_CONNECTION (nm_connection_get_setting (connection,
> NM_TYPE_SETTING_CONNECTION));
> + g_assert (s_con);
> +
> s_vpn = NM_SETTING_VPN (nm_connection_get_setting (connection,
> NM_TYPE_SETTING_VPN));
> g_assert (s_vpn);
>
> @@ -573,7 +580,7 @@ real_connect (NMVPNPlugin *plugin,
> if (getenv ("NM_VPNC_DUMP_CONNECTION") || debug)
> nm_connection_dump (connection);
>
> - if (!nm_vpnc_config_write (vpnc_fd, s_vpn, error))
> + if (!nm_vpnc_config_write (vpnc_fd, s_con, s_vpn, error))
> goto out;
>
> success = TRUE;
_______________________________________________
networkmanager-list mailing list
[email protected]
https://mail.gnome.org/mailman/listinfo/networkmanager-list
