Re: [PATCH 2/5] vlan: add ifcfg-vlan parser
- From: Dan Williams <dcbw redhat com>
- To: Weiping Pan <wpan redhat com>
- Cc: networkmanager-list gnome org
- Subject: Re: [PATCH 2/5] vlan: add ifcfg-vlan parser
- Date: Mon, 21 Nov 2011 23:16:22 -0600
On Mon, 2011-11-21 at 23:14 -0600, Dan Williams wrote:
> On Mon, 2011-11-21 at 08:20 -0500, Weiping Pan wrote:
> > The example of ifcfg-vlan is as followed:
> >
> > VLAN=yes
> > TYPE=vlan
> > DEVICE=vlan43 or "DEVICE=eth9.43"
> > PHYSDEV=eth9
> > REORDER_HDR=0
> > VLAN_FLAGS=GVRP,LOOSE_BINDING
> > VLAN_INGRESS_PRIORITY_MAP=0:1,2:5
> > VLAN_EGRESS_PRIORITY_MAP=12:3,14:7
> > ONBOOT=yes
> > BOOTPROTO=static
> > IPADDR=192.168.43.149
> > NETMASK=255.255.255.0
> >
> > And we try to make it compitable with the format used by initscripts,
> > and there is no need to change anything in ifcfg-eth9.
> >
> > Pay attention the format of DEVICE here is 'vlan+id', and id is 0-4095.
> >
> > V3:
> > 1 parse format "DEVICE=eth9.43"
> > 2 modify NMSettingVlan->get_property()
> >
> > V2:
> > 1 use "Vlan" and "vlan" to keep consistency
> > 2 remove duplicate "VLAN" or "vlan"
> > 3 add enum NMVlanFlags
> >
> > Signed-off-by: Weiping Pan <wpan redhat com>
> > ---
> > libnm-util/Makefile.am | 2 +
> > libnm-util/libnm-util.ver | 11 +
> > libnm-util/nm-connection.c | 26 +-
> > libnm-util/nm-connection.h | 2 +
> > libnm-util/nm-setting-vlan.c | 601 ++++++++++++++++++++
> > libnm-util/nm-setting-vlan.h | 118 ++++
> > src/settings/plugins/ifcfg-rh/common.h | 1 +
> > src/settings/plugins/ifcfg-rh/reader.c | 193 ++++++-
> > .../network-scripts/ifcfg-test-vlan-interface | 11 +-
> > .../plugins/ifcfg-rh/tests/test-ifcfg-rh.c | 3 +-
> > 10 files changed, 949 insertions(+), 19 deletions(-)
> > create mode 100644 libnm-util/nm-setting-vlan.c
> > create mode 100644 libnm-util/nm-setting-vlan.h
> >
> > diff --git a/libnm-util/Makefile.am b/libnm-util/Makefile.am
> > index 627c3e3..ed7134a 100644
> > --- a/libnm-util/Makefile.am
> > +++ b/libnm-util/Makefile.am
> > @@ -18,6 +18,7 @@ libnm_util_include_HEADERS = \
> > nm-setting-bond.h \
> > nm-setting-connection.h \
> > nm-setting-ip4-config.h \
> > + nm-setting-vlan.h \
> > nm-setting-ip6-config.h \
> > nm-setting-ppp.h \
> > nm-setting-pppoe.h \
> > @@ -48,6 +49,7 @@ libnm_util_la_csources = \
> > nm-setting-bond.c \
> > nm-setting-connection.c \
> > nm-setting-ip4-config.c \
> > + nm-setting-vlan.c \
> > nm-setting-ip6-config.c \
> > nm-setting-ppp.c \
> > nm-setting-pppoe.c \
> > diff --git a/libnm-util/libnm-util.ver b/libnm-util/libnm-util.ver
> > index 9b714ba..12970ad 100644
> > --- a/libnm-util/libnm-util.ver
> > +++ b/libnm-util/libnm-util.ver
> > @@ -31,6 +31,7 @@ global:
> > nm_connection_get_setting_wired;
> > nm_connection_get_setting_wireless;
> > nm_connection_get_setting_wireless_security;
> > + nm_connection_get_setting_vlan;
> > nm_connection_get_type;
> > nm_connection_get_uuid;
> > nm_connection_is_type;
> > @@ -464,6 +465,16 @@ global:
> > nm_utils_wifi_find_next_channel;
> > nm_utils_wifi_freq_to_channel;
> > nm_utils_wifi_is_channel_valid;
> > + nm_setting_vlan_error_get_type;
> > + nm_setting_vlan_error_quark;
> > + nm_setting_vlan_get_type;
> > + nm_setting_vlan_new;
> > + nm_setting_vlan_get_interface_name;
> > + nm_setting_vlan_get_slave;
> > + nm_setting_vlan_get_id;
> > + nm_setting_vlan_get_flags;
> > + nm_setting_vlan_get_ingress_priority_map;
> > + nm_setting_vlan_get_egress_priority_map;
>
> The .ver file symbols need to be kept sorted for 'make check' to work,
> so keep them where they'd normally go. If you run 'make check' in
> libnm-util after adding these it'll actually spit out a diff that'll
> tell you where they should go.
>
> > local:
> > *;
> > };
> > diff --git a/libnm-util/nm-connection.c b/libnm-util/nm-connection.c
> > index de6e948..e77f892 100644
> > --- a/libnm-util/nm-connection.c
> > +++ b/libnm-util/nm-connection.c
> > @@ -45,7 +45,7 @@
> > #include "nm-setting-vpn.h"
> > #include "nm-setting-olpc-mesh.h"
> > #include "nm-setting-bond.h"
> > -
> > +#include "nm-setting-vlan.h"
> > #include "nm-setting-serial.h"
> > #include "nm-setting-gsm.h"
> > #include "nm-setting-cdma.h"
> > @@ -136,7 +136,7 @@ static guint signals[LAST_SIGNAL] = { 0 };
> >
> > static GHashTable *registered_settings = NULL;
> >
> > -#define DEFAULT_MAP_SIZE 17
> > +#define DEFAULT_MAP_SIZE 18
> >
> > static struct SettingInfo {
> > const char *name;
> > @@ -248,6 +248,11 @@ register_default_settings (void)
> > NM_SETTING_BOND_ERROR,
> > 1, TRUE);
> >
> > + register_one_setting (NM_SETTING_VLAN_SETTING_NAME,
> > + NM_TYPE_SETTING_VLAN,
> > + NM_SETTING_VLAN_ERROR,
> > + 1, TRUE);
> > +
> > register_one_setting (NM_SETTING_WIRELESS_SECURITY_SETTING_NAME,
> > NM_TYPE_SETTING_WIRELESS_SECURITY,
> > NM_SETTING_WIRELESS_SECURITY_ERROR,
> > @@ -1582,6 +1587,23 @@ nm_connection_get_setting_wireless_security (NMConnection *connection)
> > return (NMSettingWirelessSecurity *) nm_connection_get_setting (connection, NM_TYPE_SETTING_WIRELESS_SECURITY);
> > }
> >
> > +/**
> > + * nm_connection_get_setting_vlan:
> > + * @connection: the #NMConnection
> > + *
> > + * A shortcut to return any #NMSettingVlan the connection might contain.
> > + *
> > + * Returns: (transfer none): an #NMSettingVlan if the connection contains one, otherwise NULL
> > + **/
> > +NMSettingVlan *
> > +nm_connection_get_setting_vlan (NMConnection *connection)
> > +{
> > + g_return_val_if_fail (connection != NULL, NULL);
> > + g_return_val_if_fail (NM_IS_CONNECTION (connection), NULL);
> > +
> > + return (NMSettingVlan *) nm_connection_get_setting (connection, NM_TYPE_SETTING_VLAN);
> > +}
> > +
> > /*************************************************************/
> >
> > static void
> > diff --git a/libnm-util/nm-connection.h b/libnm-util/nm-connection.h
> > index 6014138..c68f7ff 100644
> > --- a/libnm-util/nm-connection.h
> > +++ b/libnm-util/nm-connection.h
> > @@ -46,6 +46,7 @@
> > #include <nm-setting-wired.h>
> > #include <nm-setting-wireless.h>
> > #include <nm-setting-wireless-security.h>
> > +#include <nm-setting-vlan.h>
> >
> > G_BEGIN_DECLS
> >
> > @@ -198,6 +199,7 @@ NMSettingWimax * nm_connection_get_setting_wimax (NMConnec
> > NMSettingWired * nm_connection_get_setting_wired (NMConnection *connection);
> > NMSettingWireless * nm_connection_get_setting_wireless (NMConnection *connection);
> > NMSettingWirelessSecurity *nm_connection_get_setting_wireless_security (NMConnection *connection);
> > +NMSettingVlan * nm_connection_get_setting_vlan (NMConnection *connection);
> >
> > G_END_DECLS
> >
> > diff --git a/libnm-util/nm-setting-vlan.c b/libnm-util/nm-setting-vlan.c
> > new file mode 100644
> > index 0000000..cc78789
> > --- /dev/null
> > +++ b/libnm-util/nm-setting-vlan.c
> > @@ -0,0 +1,601 @@
> > +/* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- */
> > +
> > +/*
> > + * Weiping Pan <wpan redhat com>
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2 of the License, or (at your option) any later version.
> > + *
> > + * This library is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; if not, write to the
> > + * Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
> > + * Boston, MA 02110-1301 USA.
> > + *
> > + * (C) Copyright 2011 Red Hat, Inc.
> > + */
> > +#include <stdio.h>
> > +#include <string.h>
> > +
> > +#include <dbus/dbus-glib.h>
> > +#include "nm-setting-vlan.h"
> > +#include "nm-param-spec-specialized.h"
> > +#include "nm-utils.h"
> > +#include "nm-dbus-glib-types.h"
> > +
> > +/**
> > + * SECTION:nm-setting-vlan
> > + * @short_description: Describes connection properties for VLAN devices
> > + * @include: nm-setting-vlan.h
> > + *
> > + * The #NMSettingVlan object is a #NMSetting subclass that describes properties
> > + * necessary for connection to VLAN devices.
> > + **/
> > +
> > +/**
> > + * nm_setting_vlan_error_quark:
> > + * Registers an error quark for #NMSettingVlan if necessary.
> > + * Returns: the error quark used for #NMSettingVlan errors.
> > + **/
> > +GQuark
> > +nm_setting_vlan_error_quark (void)
> > +{
> > + static GQuark quark;
> > +
> > + if (G_UNLIKELY (!quark))
> > + quark = g_quark_from_static_string ("nm-setting-vlan-error-quark");
> > + return quark;
> > +}
> > +
> > +/* This should really be standard. */
> > +#define ENUM_ENTRY(NAME, DESC) { NAME, "" #NAME "", DESC }
> > +
> > +GType
> > +nm_setting_vlan_error_get_type (void)
> > +{
> > + static GType etype = 0;
> > +
> > + if (etype == 0) {
> > + static const GEnumValue values[] = {
> > + /* Unknown error. */
> > + ENUM_ENTRY (NM_SETTING_VLAN_ERROR_UNKNOWN, "UnknownError"),
> > + /* The specified property was invalid. */
> > + ENUM_ENTRY (NM_SETTING_VLAN_ERROR_INVALID_PROPERTY, "InvalidProperty"),
> > + /* The specified property was missing and is required. */
> > + ENUM_ENTRY (NM_SETTING_VLAN_ERROR_MISSING_PROPERTY, "MissingProperty"),
> > + { 0, 0, 0 }
> > + };
> > + etype = g_enum_register_static ("NMSettingVlanError", values);
> > + }
> > + return etype;
> > +}
> > +
> > +G_DEFINE_TYPE (NMSettingVlan, nm_setting_vlan, NM_TYPE_SETTING)
> > +
> > +#define NM_SETTING_VLAN_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), NM_TYPE_SETTING_VLAN, NMSettingVlanPrivate))
> > +
> > +typedef struct {
> > + char *interface_name;
> > + char *vlan_slave;
> > + guint32 vlan_id;
> > + guint32 vlan_flags;
> > + GSList *vlan_priority_ingress_map;
> > + GSList *vlan_priority_egress_map;
> > +} NMSettingVlanPrivate;
> > +
> > +enum {
> > + PROP_0,
> > + PROP_VLAN_DEVICE,
> > + PROP_VLAN_SLAVE,
> > + PROP_VLAN_ID,
> > + PROP_VLAN_FLAGS,
> > + PROP_VLAN_NAME_TYPE,
> > + PROP_VLAN_PRIORITY_INGRESS_MAP,
> > + PROP_VLAN_PRIORITY_EGRESS_MAP,
> > + LAST_PROP
> > +};
> > +
> > +/**
> > + * nm_setting_vlan_new:
> > + * Creates a new #NMSettingVlan object with default values.
> > + *
> > + * Returns: (transfer full): the new empty #NMSettingVlan object
> > + **/
> > +NMSetting *
> > +nm_setting_vlan_new (void)
> > +{
> > + return (NMSetting *) g_object_new (NM_TYPE_SETTING_VLAN, NULL);
> > +}
> > +
> > +/**
> > + * nm_setting_vlan_get_interface_name:
> > + * @setting: the #NMSettingVlan
> > + *
> > + * Returns: the #NMSettingVlan:interface_name property of the setting
> > + **/
> > +const char *
> > +nm_setting_vlan_get_interface_name (NMSettingVlan *setting)
> > +{
> > + g_return_val_if_fail (NM_IS_SETTING_VLAN (setting), NULL);
> > + return NM_SETTING_VLAN_GET_PRIVATE (setting)->interface_name;
> > +}
> > +
> > +/**
> > + * nm_setting_vlan_get_slave:
> > + * @setting: the #NMSettingVlan
> > + *
> > + * Returns: the #NMSettingVlan:vlan_slave property of the setting
> > + **/
> > +const char *
> > +nm_setting_vlan_get_slave (NMSettingVlan *setting)
> > +{
> > + g_return_val_if_fail (NM_IS_SETTING_VLAN (setting), NULL);
> > + return NM_SETTING_VLAN_GET_PRIVATE (setting)->vlan_slave;
> > +}
> > +
> > +/**
> > + * nm_setting_vlan_get_id:
> > + * @setting: the #NMSettingVlan
> > + *
> > + * Returns: the #NMSettingVlan:vlan_id property of the setting
> > + **/
> > +guint32
> > +nm_setting_vlan_get_id (NMSettingVlan *setting)
> > +{
> > + g_return_val_if_fail (NM_IS_SETTING_VLAN (setting), 0);
> > + return NM_SETTING_VLAN_GET_PRIVATE (setting)->vlan_id;
> > +}
> > +
> > +/**
> > + * nm_setting_vlan_get_flags:
> > + * @setting: the #NMSettingVlan
> > + *
> > + * Returns: the #NMSettingVlan:vlan_flags property of the setting
> > + **/
> > +guint32
> > +nm_setting_vlan_get_flags (NMSettingVlan *setting)
> > +{
> > + g_return_val_if_fail (NM_IS_SETTING_VLAN (setting), 0);
> > + return NM_SETTING_VLAN_GET_PRIVATE (setting)->vlan_flags;
> > +}
> > +
> > +/**
> > + * nm_setting_vlan_get_ingress_priority_map:
> > + * @setting: the #NMSettingVlan
> > + *
> > + * Returns: the #NMSettingVlan:vlan_priority_ingress_map property of the setting
> > + **/
> > +const GSList *
> > +nm_setting_vlan_get_ingress_priority_map(NMSettingVlan *setting)
> > +{
> > + g_return_val_if_fail (NM_IS_SETTING_VLAN (setting), NULL);
> > +
> > + return NM_SETTING_VLAN_GET_PRIVATE (setting)->vlan_priority_ingress_map;
> > +}
Forgot one thing; lets get rid of the functions here which return the
GSList of structures, since I think we'd rather just use the accessors
below to ensure that we can extend the internal ingress/egress priority
map in the future and that the structure is an implementation detail.
> > +
> > +/**
> > + * nm_setting_vlan_get_egress_priority_map:
> > + * @setting: the #NMSettingVlan
> > + *
> > + * Returns: the #NMSettingVlan:vlan_priority_egress_map property of the setting
> > + **/
> > +const GSList *
> > +nm_setting_vlan_get_egress_priority_map(NMSettingVlan *setting)
> > +{
> > + g_return_val_if_fail (NM_IS_SETTING_VLAN (setting), NULL);
> > +
> > + return NM_SETTING_VLAN_GET_PRIVATE (setting)->vlan_priority_egress_map;
> > +}
> > +
> > +static void
> > +nm_setting_vlan_init (NMSettingVlan *setting)
> > +{
> > + NMSettingVlanPrivate *priv = NULL;
> > +
> > + priv = NM_SETTING_VLAN_GET_PRIVATE(setting);
> > + priv->vlan_id = 0;
> > + priv->vlan_flags = 0;
> > +
> > + priv->vlan_priority_ingress_map = NULL;
> > + priv->vlan_priority_egress_map = NULL;
>
> The private data is actually zeroed already, so we don't need to
> initialize anything to NULL/0 here.
>
> > + g_object_set (setting, NM_SETTING_NAME, NM_SETTING_VLAN_SETTING_NAME, NULL);
> > +
> > + return;
> > +}
> > +
> > +static void
> > +finalize (GObject *object)
> > +{
> > + NMSettingVlan *setting = NM_SETTING_VLAN (object);
> > + NMSettingVlanPrivate *priv = NM_SETTING_VLAN_GET_PRIVATE (setting);
> > +
> > + g_free (priv->interface_name);
> > + g_free (priv->vlan_slave);
> > + nm_utils_slist_free (priv->vlan_priority_ingress_map, g_free);
> > + nm_utils_slist_free (priv->vlan_priority_egress_map, g_free);
> > + return;
> > +}
> > +
> > +static vlan_priority_map*
> > +priority_map_new_from_str (const char *str)
> > +{
> > + vlan_priority_map *p = NULL;
> > + guint32 from = 0, to = 0;
> > + gchar **t = NULL, *key = NULL, *value = NULL;
> > +
> > + if (!str)
> > + return NULL;
> > + p = g_malloc0 (sizeof (vlan_priority_map));
> > + if (!p)
> > + return NULL;
> > +
> > + t = g_strsplit (str, ":", 0);
>
> You might want to check that 't' is at least 2 elements long, either
> with g_strv_length() or (t[0] != NULL && t[1] != NULL). We can't
> necessarily trust the incoming string to be correctly formatted.
>
> > + key = *t;
> > + value = *(t+1);
> > + from = g_ascii_strtoull (key, NULL, 10);
> > + to = g_ascii_strtoull (value, NULL, 10);
>
> Could just do:
>
> p->from = g_ascii_strtoull(xxx)
> p->to = g_ascii_strtoull(xxx)
>
> > + p->from = from;
> > + p->to = to;
>
> and skip this assignment.
>
> > +
> > + g_strfreev(t);
> > +
> > + return p;
> > +}
> > +
> > +static GSList *
> > +priority_stringlist_to_maplist (GSList *strlist)
> > +{
> > + GSList *list = NULL, *iter;
> > +
> > + for (iter = strlist; iter; iter = g_slist_next (iter)) {
> > + vlan_priority_map *item = NULL;
> > + item = priority_map_new_from_str ((const char *) iter->data);
> > + if (item)
> > + list = g_slist_append (list, item);
> > + }
> > +
> > + return list;
> > +}
> > +
> > +static GSList *
> > +priority_maplist_to_stringlist (GSList *list)
> > +{
> > + GSList *strlist = NULL, *iterator;
> > + GString *text = NULL;
> > + vlan_priority_map *item = NULL;
> > +
> > + text = g_string_new("");
> > + for (iterator = list; iterator; iterator = iterator->next) {
> > + item = (vlan_priority_map*)(iterator->data);
> > + g_string_printf(text, "%d:%d", item->from, item->to);
> > + strlist = g_slist_append(strlist, text->str);
>
> Here, I'd say skip the GString, it's more work than it's worth, and you
> have to remember to free the GString structure too. So instead, just do
> something like:
>
> for (iterator = list; iterator; iterator = iterater->next) {
> item = iterator->data;
> strlist = g_slist_append (strlist, g_strdup_printf ("%d:%d", item->from, item->to);
> }
>
> otherwise you've got to use g_string_free (xxx, TRUE); to free the
> GString structure, but not free the string data. More complicated.
>
> > + }
> > +
> > + return strlist;
> > +}
> > +
> > +static void
> > +set_property (GObject *object, guint prop_id,
> > + const GValue *value, GParamSpec *pspec)
> > +{
> > + NMSettingVlan *setting = NM_SETTING_VLAN (object);
> > + NMSettingVlanPrivate *priv = NM_SETTING_VLAN_GET_PRIVATE (setting);
> > +
> > + switch (prop_id) {
> > + case PROP_VLAN_DEVICE:
> > + g_free (priv->interface_name);
> > + priv->interface_name = g_value_dup_string (value);
> > + break;
> > + case PROP_VLAN_SLAVE:
> > + g_free (priv->vlan_slave);
> > + priv->vlan_slave = g_value_dup_string (value);
> > + break;
> > + case PROP_VLAN_ID:
> > + priv->vlan_id = g_value_get_uint(value);
> > + break;
> > + case PROP_VLAN_FLAGS:
> > + priv->vlan_flags = g_value_get_uint(value);
> > + break;
> > + case PROP_VLAN_PRIORITY_INGRESS_MAP:
> > + nm_utils_slist_free (priv->vlan_priority_ingress_map, g_free);
> > + priv->vlan_priority_ingress_map =
> > + priority_stringlist_to_maplist (g_value_dup_boxed (value));
>
> Here, g_value_dup_boxed() will actually duplicate the incoming string
> list, and it's not getting freed. But we don't actually need to
> duplicate it; instead just use g_value_get_boxed() since the
> priority_stringlist_to_maplist() function doesn't need to modify the
> incoming string list.
>
> > + break;
> > + case PROP_VLAN_PRIORITY_EGRESS_MAP:
> > + nm_utils_slist_free (priv->vlan_priority_egress_map, g_free);
> > + priv->vlan_priority_egress_map =
> > + priority_stringlist_to_maplist (g_value_dup_boxed (value));
>
> Same here, g_value_get_boxed().
>
> > + break;
> > + default:
> > + break;
> > + }
> > +}
> > +
> > +static void
> > +get_property (GObject *object, guint prop_id,
> > + GValue *value, GParamSpec *pspec)
> > +{
> > + NMSettingVlan *setting = NM_SETTING_VLAN (object);
> > + NMSettingVlanPrivate *priv = NM_SETTING_VLAN_GET_PRIVATE (setting);
> > +
> > + switch (prop_id) {
> > + case PROP_VLAN_DEVICE:
> > + g_value_set_string (value, nm_setting_vlan_get_interface_name (setting));
> > + break;
> > + case PROP_VLAN_SLAVE:
> > + g_value_set_string (value, nm_setting_vlan_get_slave (setting));
> > + break;
> > + case PROP_VLAN_ID:
> > + g_value_set_uint (value, nm_setting_vlan_get_id (setting));
> > + break;
> > + case PROP_VLAN_FLAGS:
> > + g_value_set_uint (value, nm_setting_vlan_get_flags (setting));
> > + break;
> > + case PROP_VLAN_PRIORITY_INGRESS_MAP:
> > + g_value_set_boxed (value, priority_maplist_to_stringlist(priv->vlan_priority_ingress_map));
>
> In the same way as above, you want g_value_take_boxed() here so that the
> GValue takes ownership over the allocated value that
> priority_maplist_to_stringlist() returns. Otherwise we leak the
> returned stringlist.
>
> > + break;
> > + case PROP_VLAN_PRIORITY_EGRESS_MAP:
> > + g_value_set_boxed (value, priority_maplist_to_stringlist(priv->vlan_priority_egress_map));
>
> Same here; g_value_take_boxed().
>
> > + break;
> > + default:
> > + break;
> > + }
> > +}
> > +
> > +/**
> > + * nm_setting_vlan_get_num_priorities:
> > + * @map: the type of priority map
> > + * @setting: the #NMSettingVlan
> > + *
> > + * Returns the number of entires in the
> > + * #NMSettingVlan:vlan_priority_ingress_map/vlan_priority_egress_map property of this setting.
> > + *
> > + * Returns: return the number of ingress/egress priority entries, -1 if error
> > + **/
> > +gint32 nm_setting_vlan_get_num_priorities (NM_VLAN_PRIORITY_MAP map,
> > + NMSettingVlan *setting)
> > +{
> > + NMSettingVlanPrivate *priv = NM_SETTING_VLAN_GET_PRIVATE (setting);
> > + GSList *list = NULL;
> > +
> > + g_return_val_if_fail (NM_IS_SETTING_VLAN (setting), FALSE);
> > +
> > + if (map == NM_VLAN_INGRESS_MAP)
> > + list = priv->vlan_priority_ingress_map;
> > + else if (map == NM_VLAN_EGRESS_MAP)
> > + list = priv->vlan_priority_egress_map;
> > + else
> > + return -1;
> > +
> > + return g_slist_length (list);
> > +}
> > +
> > +/**
> > + * nm_setting_vlan_get_priority:
> > + * @map: the type of priority map
> > + * @setting: the #NMSettingVlan
> > + * @idx: the zero-based index of the ingress/egress priority map entry
> > + * @from: on return, the value of vlan_priority_map->from
> > + * @to: on return, the value of vlan_priority_map->to
> > + *
> > + * Retrieve one of the entries of the
> > + * #NMSettingVlan:vlan_priority_ingress_map/vlan_priority_egress_map property of this setting.
> > + *
> > + * Returns: %TRUE if a priority map was returned, %FALSE if error
> > + **/
> > +gboolean nm_setting_vlan_get_priority (NM_VLAN_PRIORITY_MAP map,
> > + NMSettingVlan *setting, guint32 idx,
> > + guint32 *from, guint32 *to)
> > +{
> > + NMSettingVlanPrivate *priv = NM_SETTING_VLAN_GET_PRIVATE (setting);
> > + GSList *list = NULL;
> > + vlan_priority_map *item = NULL;
> > +
> > + g_return_val_if_fail (NM_IS_SETTING_VLAN (setting), FALSE);
> > + g_return_val_if_fail (from != NULL, FALSE);
> > + g_return_val_if_fail (to != NULL, FALSE);
> > +
> > + if (map == NM_VLAN_INGRESS_MAP)
> > + list = priv->vlan_priority_ingress_map;
> > + else if (map == NM_VLAN_EGRESS_MAP)
> > + list = priv->vlan_priority_egress_map;
> > + else
> > + return FALSE;
> > +
> > + g_return_val_if_fail (idx < g_slist_length(list), FALSE);
> > +
> > + item = (vlan_priority_map *) g_slist_nth_data (list, idx);
>
> To save some code, all the g_slist/g_list/etc functions take and return
> gpointer (ie, void*) so you can skip the casting here and let the
> compiler do it for you if you want.
>
> > + if (item) {
> > + *from = item->from;
> > + *to = item->to;
> > + return TRUE;
> > + }
> > +
> > + return FALSE;
> > +}
> > +
> > +/**
> > + * nm_setting_vlan_add_priority:
> > + * @map: the type of priority map
> > + * @setting: the #NMSettingVlan
> > + * @from: the value of vlan_priority_map->from
> > + * @to: the value of vlan_priority_map->to
> > + *
> > + * Adds a vlan_priority_map to
> > + * #NMSettingVlan:vlan_priority_ingress_map/vlan_priority_egress_map list.
>
> It looks like if you call this function for a 'from' that's already
> known, it'll update the 'to' value with whatever you pass in. Would be
> good to note that in the docs here.
>
> > + *
> > + * Returns: TRUE if the a vlan_priority_map was successfully added to the
> > + * list, FALSE if error
> > + */
> > +gboolean nm_setting_vlan_add_priority (NM_VLAN_PRIORITY_MAP map,
> > + NMSettingVlan *setting, guint32 from, guint32 to)
> > +{
> > + NMSettingVlanPrivate *priv = NM_SETTING_VLAN_GET_PRIVATE (setting);
> > + GSList *list = NULL, *iterator = NULL;
> > + vlan_priority_map *item = NULL;
> > +
> > + g_return_val_if_fail (NM_IS_SETTING_VLAN (setting), FALSE);
> > +
> > + if (map == NM_VLAN_INGRESS_MAP)
> > + list = priv->vlan_priority_ingress_map;
> > + else if (map == NM_VLAN_EGRESS_MAP)
> > + list = priv->vlan_priority_egress_map;
> > + else
> > + return FALSE;
> > +
> > + for (iterator = list; iterator; iterator = iterator->next) {
> > + item = (vlan_priority_map*)(iterator->data);
>
> As above, iterator->data is a void* so you can skip the explicit cast if
> you like.
>
> > + if (item->from == from) {
> > + item->to = to;
> > + return TRUE;
> > + }
> > + }
> > +
> > + item = g_malloc0 (sizeof(vlan_priority_map));
> > + g_return_val_if_fail(item != NULL, FALSE);
> > +
> > + item->from = from;
> > + item->to = to;
> > +
> > + list = g_slist_append (list, (gpointer)item);
>
> Again, shouldn't need explicit casts for g_slist_xxx().
>
> > +
> > + return TRUE;
> > +}
> > +
> > +/**
> > + * nm_setting_vlan_remove_priority:
> > + * @map: the type of priority map
> > + * @setting: the #NMSettingVlan
> > + * @idx: the zero-based index of the priority map to remove
> > + *
> > + * Removes the priority map at index @idx from
> > + * #NMSettingVlan:vlan_priority_ingress_map/vlan_priority_egress_map list.
> > + */
> > +void nm_setting_vlan_remove_priority (NM_VLAN_PRIORITY_MAP map,
> > + NMSettingVlan *setting, guint32 idx)
> > +{
> > + NMSettingVlanPrivate *priv = NM_SETTING_VLAN_GET_PRIVATE (setting);
> > + GSList *list = NULL, *item = NULL;
> > +
> > + g_return_if_fail (NM_IS_SETTING_VLAN (setting));
> > +
> > + if (map == NM_VLAN_INGRESS_MAP)
> > + list = priv->vlan_priority_ingress_map;
> > + else if (map == NM_VLAN_EGRESS_MAP)
> > + list = priv->vlan_priority_egress_map;
> > + else
> > + return;
> > +
> > + g_return_if_fail (idx < g_slist_length (list));
> > + item = g_slist_nth_data (list, idx);
> > + g_free (item->data);
> > + list = g_slist_delete_link (list, item);
> > +
> > + return;
> > +}
> > +
> > +/**
> > + * nm_setting_vlan_remove_priority:
>
> Wrong function doc here, should be nm_setting_vlan_clear_priorities().
>
> > + * @map: the type of priority map
> > + * @setting: the #NMSettingVlan
> > + *
> > + * Clear all the entires from
> > + * #NMSettingVlan:vlan_priority_ingress_map/vlan_priority_egress_map list.
> > + */
> > +void nm_setting_vlan_clear_priorities (NM_VLAN_PRIORITY_MAP map,
> > + NMSettingVlan *setting)
> > +{
> > + NMSettingVlanPrivate *priv = NM_SETTING_VLAN_GET_PRIVATE (setting);
> > + GSList *list = NULL;
> > +
> > + if (!priv)
> > + return;
> > +
> > + if (map == NM_VLAN_INGRESS_MAP)
> > + list = priv->vlan_priority_ingress_map;
> > + else if (map == NM_VLAN_EGRESS_MAP)
> > + list = priv->vlan_priority_egress_map;
> > + else
> > + return;
> > +
> > + nm_utils_slist_free (list, g_free);
> > +
> > + if (map == NM_VLAN_INGRESS_MAP)
> > + priv->vlan_priority_ingress_map = NULL;
> > + else if (map == NM_VLAN_EGRESS_MAP)
> > + priv->vlan_priority_egress_map = NULL;
> > +
> > + return;
> > +}
> > +
> > +static void
> > +nm_setting_vlan_class_init (NMSettingVlanClass *setting_class)
> > +{
> > + GObjectClass *object_class = G_OBJECT_CLASS (setting_class);
> > + g_type_class_add_private (setting_class, sizeof(NMSettingVlanPrivate));
> > +
> > + /* virtual methods */
> > + object_class->set_property = set_property;
> > + object_class->get_property = get_property;
> > + object_class->finalize = finalize;
> > +
> > + /* Properties */
> > + g_object_class_install_property
> > + (object_class, PROP_VLAN_DEVICE,
> > + g_param_spec_string (NM_SETTING_VLAN_INTERFACE_NAME,
> > + "InterfaceName",
> > + "The vlan device name in kernel",
> > + NULL,
> > + G_PARAM_READWRITE | G_PARAM_CONSTRUCT | NM_SETTING_PARAM_SERIALIZE));
> > +
> > + g_object_class_install_property
> > + (object_class, PROP_VLAN_SLAVE,
> > + g_param_spec_string (NM_SETTING_VLAN_SLAVE,
> > + "vlan slave",
> > + "The underlying physical ethernet device",
> > + NULL,
> > + G_PARAM_READWRITE | G_PARAM_CONSTRUCT | NM_SETTING_PARAM_SERIALIZE));
> > +
> > + g_object_class_install_property
> > + (object_class, PROP_VLAN_ID,
> > + g_param_spec_uint (NM_SETTING_VLAN_ID,
> > + "vlan id",
> > + "vlan id",
> > + 0,
> > + G_MAXUINT32,
> > + 0,
> > + G_PARAM_READWRITE | G_PARAM_CONSTRUCT | NM_SETTING_PARAM_SERIALIZE));
> > +
> > + g_object_class_install_property
> > + (object_class, PROP_VLAN_FLAGS,
> > + g_param_spec_uint (NM_SETTING_VLAN_FLAGS,
> > + "vlan flags",
> > + "vlan flags",
> > + 0,
> > + G_MAXUINT32,
> > + 0,
> > + G_PARAM_READWRITE | G_PARAM_CONSTRUCT | NM_SETTING_PARAM_SERIALIZE));
> > +
> > + g_object_class_install_property
> > + (object_class, PROP_VLAN_PRIORITY_INGRESS_MAP,
> > + _nm_param_spec_specialized (NM_SETTING_VLAN_INGRESS_PRIORITY_MAP,
> > + "vlan ingress priority mapping",
> > + "vlan ingress priority mapping",
> > + DBUS_TYPE_G_LIST_OF_STRING,
> > + G_PARAM_READWRITE | NM_SETTING_PARAM_SERIALIZE));
> > +
> > + g_object_class_install_property
> > + (object_class, PROP_VLAN_PRIORITY_EGRESS_MAP,
> > + _nm_param_spec_specialized (NM_SETTING_VLAN_EGRESS_PRIORITY_MAP,
> > + "vlan egress priority mapping",
> > + "vlan egress priority mapping",
> > + DBUS_TYPE_G_LIST_OF_STRING,
> > + G_PARAM_READWRITE | NM_SETTING_PARAM_SERIALIZE));
> > +}
> > diff --git a/libnm-util/nm-setting-vlan.h b/libnm-util/nm-setting-vlan.h
> > new file mode 100644
> > index 0000000..7ec05ba
> > --- /dev/null
> > +++ b/libnm-util/nm-setting-vlan.h
> > @@ -0,0 +1,118 @@
> > +/* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- */
> > +
> > +/*
> > + * Weiping Pan <wpan redhat com>
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2 of the License, or (at your option) any later version.
> > + *
> > + * This library is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; if not, write to the
> > + * Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
> > + * Boston, MA 02110-1301 USA.
> > + *
> > + * (C) Copyright 2011 Red Hat, Inc.
> > + */
> > +
> > +#ifndef NM_SETTING_VLAN_H
> > +#define NM_SETTING_VLAN_H
> > +
> > +#include "nm-setting.h"
> > +#include <linux/if_vlan.h>
> > +
> > +G_BEGIN_DECLS
> > +
> > +#define NM_TYPE_SETTING_VLAN (nm_setting_vlan_get_type ())
> > +#define NM_SETTING_VLAN(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), NM_TYPE_SETTING_VLAN, NMSettingVlan))
> > +#define NM_SETTING_VLAN_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST ((klass), NM_TYPE_SETTING_VLANCONFIG, NMSettingVlanClass))
> > +#define NM_IS_SETTING_VLAN(obj) (G_TYPE_CHECK_INSTANCE_TYPE ((obj), NM_TYPE_SETTING_VLAN))
> > +#define NM_IS_SETTING_VLAN_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((obj), NM_TYPE_SETTING_VLAN))
> > +#define NM_SETTING_VLAN_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), NM_TYPE_SETTING_VLAN, NMSettingVlanClass))
> > +
> > +#define NM_SETTING_VLAN_SETTING_NAME "vlan"
> > +
> > +/**
> > + * NMSettingVlanError:
> > + * @NM_SETTING_VLAN_ERROR_UNKNOWN: unknown or unclassified error
> > + * @NM_SETTING_VLAN_ERROR_INVALID_PROPERTY: the property was invalid
> > + * @NM_SETTING_VLAN_ERROR_MISSING_PROPERTY: the property was missing and is
> > + * required
> > + */
> > +typedef enum {
> > + NM_SETTING_VLAN_ERROR_UNKNOWN = 0,
> > + NM_SETTING_VLAN_ERROR_INVALID_PROPERTY,
> > + NM_SETTING_VLAN_ERROR_MISSING_PROPERTY
> > +} NMSettingVlanError;
> > +
> > +#define NM_TYPE_SETTING_VLAN_ERROR (nm_setting_vlan_error_get_type ())
> > +GType nm_setting_vlan_error_get_type (void);
> > +
> > +#define NM_SETTING_VLAN_ERROR nm_setting_vlan_error_quark ()
> > +GQuark nm_setting_vlan_error_quark (void);
> > +
> > +GType nm_setting_vlan_get_type (void);
> > +
> > +#define NM_SETTING_VLAN_INTERFACE_NAME "interface-name"
> > +#define NM_SETTING_VLAN_SLAVE "slave"
> > +#define NM_SETTING_VLAN_ID "id"
> > +#define NM_SETTING_VLAN_FLAGS "flags"
> > +#define NM_SETTING_VLAN_INGRESS_PRIORITY_MAP "priority-ingress-map"
> > +#define NM_SETTING_VLAN_EGRESS_PRIORITY_MAP "priority-egress-map"
> > +
> > +typedef struct {
> > + NMSetting parent;
> > +} NMSettingVlan;
> > +
> > +typedef struct {
> > + NMSettingClass parent;
> > +
> > + /* Padding for future expansion */
> > + void (*_reserved1) (void);
> > + void (*_reserved2) (void);
> > + void (*_reserved3) (void);
> > + void (*_reserved4) (void);
> > +} NMSettingVlanClass;
> > +
> > +typedef enum {
> > + NM_VLAN_INGRESS_MAP,
> > + NM_VLAN_EGRESS_MAP
> > +} NM_VLAN_PRIORITY_MAP;
>
> Could we perhaps make the enum name StudlyCaps ie NMVlanPriorityMap?
> Stylistic thing that matches what we do for other enums in NM.
>
> > +
> > +typedef struct {
> > + guint32 from;
> > + guint32 to;
> > +} vlan_priority_map;
>
> This struct shouldn't be in the public header since it's a private
> internal implementation detail; we can just put it near the top of
> nm-setting-vlan.c.
>
> > +
> > +enum NMVlanFlags {
> > + NM_VLAN_FLAG_REORDER_HDR = 0x1,
> > + NM_VLAN_FLAG_GVRP = 0x2,
> > + NM_VLAN_FLAG_LOOSE_BINDING = 0x4,
> > +};
> > +
> > +NMSetting *nm_setting_vlan_new (void);
> > +const char *nm_setting_vlan_get_interface_name (NMSettingVlan *setting);
> > +const char *nm_setting_vlan_get_slave (NMSettingVlan *setting);
> > +guint32 nm_setting_vlan_get_id (NMSettingVlan *setting);
> > +guint32 nm_setting_vlan_get_flags (NMSettingVlan *setting);
> > +
> > +const GSList *nm_setting_vlan_get_ingress_priority_map (NMSettingVlan *setting);
> > +const GSList *nm_setting_vlan_get_egress_priority_map (NMSettingVlan *setting);
> > +
> > +gint32 nm_setting_vlan_get_num_priorities (NM_VLAN_PRIORITY_MAP map, NMSettingVlan *setting);
> > +gboolean nm_setting_vlan_get_priority (NM_VLAN_PRIORITY_MAP map, NMSettingVlan *setting, guint32 idx,
> > + guint32 *out_a, guint32 *out_b);
> > +gboolean nm_setting_vlan_add_priority (NM_VLAN_PRIORITY_MAP map, NMSettingVlan *setting, guint32 a, guint32 b);
> > +void nm_setting_vlan_remove_priority (NM_VLAN_PRIORITY_MAP map, NMSettingVlan *setting, guint32 idx);
> > +void nm_setting_vlan_clear_priorities (NM_VLAN_PRIORITY_MAP map, NMSettingVlan *setting);
> > +
> > +G_END_DECLS
> > +
> > +#endif /* NM_SETTING_VLAN_H */
> > diff --git a/src/settings/plugins/ifcfg-rh/common.h b/src/settings/plugins/ifcfg-rh/common.h
> > index d801be1..546deda 100644
> > --- a/src/settings/plugins/ifcfg-rh/common.h
> > +++ b/src/settings/plugins/ifcfg-rh/common.h
> > @@ -45,6 +45,7 @@
> > #define TYPE_WIRELESS "Wireless"
> > #define TYPE_BRIDGE "Bridge"
> > #define TYPE_BOND "Bond"
> > +#define TYPE_VLAN "Vlan"
> >
> > #define SECRET_FLAG_AGENT "user"
> > #define SECRET_FLAG_NOT_SAVED "ask"
> > diff --git a/src/settings/plugins/ifcfg-rh/reader.c b/src/settings/plugins/ifcfg-rh/reader.c
> > index ea31f16..1e8d5f5 100644
> > --- a/src/settings/plugins/ifcfg-rh/reader.c
> > +++ b/src/settings/plugins/ifcfg-rh/reader.c
> > @@ -19,6 +19,7 @@
> > */
> >
> > #include <config.h>
> > +#include <stdio.h>
> > #include <stdlib.h>
> > #include <string.h>
> > #include <sys/types.h>
> > @@ -46,6 +47,7 @@
> > #include <NetworkManager.h>
> > #include <nm-setting-connection.h>
> > #include <nm-setting-ip4-config.h>
> > +#include <nm-setting-vlan.h>
> > #include <nm-setting-ip6-config.h>
> > #include <nm-setting-wired.h>
> > #include <nm-setting-wireless.h>
> > @@ -3307,6 +3309,179 @@ wired_connection_from_ifcfg (const char *file,
> > return connection;
> > }
> >
> > +static NMSetting *
> > +make_vlan_setting (shvarFile *ifcfg,
> > + const char *file,
> > + gboolean nm_controlled,
> > + char **unmanaged,
> > + NMSetting8021x **s_8021x,
> > + GError **error)
> > +{
> > + NMSettingVlan *s_vlan = NULL;
> > + char *value = NULL;
> > + char *interface_name = NULL;
> > + char *vlan_slave = NULL;
> > + char *p = NULL;
> > + guint32 vlan_id = 0;
> > + guint32 vlan_flags = 0;
> > + GSList *vlan_priority_ingress_map = NULL;
> > + GSList *vlan_priority_egress_map = NULL;
> > +
> > +
> > + s_vlan = NM_SETTING_VLAN(nm_setting_vlan_new());
> > +
> > + interface_name = svGetValue (ifcfg, "DEVICE", FALSE);
> > + if (!interface_name)
> > + goto error_return;
> > + else {
> > + g_object_set(s_vlan, NM_SETTING_VLAN_INTERFACE_NAME, interface_name, NULL);
> > +
> > + p = g_utf8_strchr(interface_name, -1, '.');
> > + if (p) {
> > + /* eth0.43 */
> > + p++;
> > + } else {
> > + /* vlan43 */
> > + p = interface_name + 4;
> > + }
> > + vlan_id = g_ascii_strtoull(p, NULL, 10);
> > + if (vlan_id >= 0 && vlan_id < 4096)
>
> If the VLAN ID is always going to be less than 4096, then we can change
> the GObject property creator in nm_setting_vlan_class_init() to clamp
> the upper bound at 4095 for the 'id' property instead of using
> G_MAXUINT32.
>
> > + g_object_set(s_vlan, NM_SETTING_VLAN_ID, vlan_id, NULL);
> > + else
> > + goto free_interface_name;
> > + }
> > +
> > + vlan_slave = svGetValue (ifcfg, "PHYSDEV", FALSE);
> > + if (vlan_slave) {
> > + g_object_set(s_vlan, NM_SETTING_VLAN_SLAVE, vlan_slave, NULL);
> > + g_free(vlan_slave);
> > + }
> > +
> > + value = svGetValue (ifcfg, "REORDER_HDR", FALSE);
> > + if (value)
> > + vlan_flags |= NM_VLAN_FLAG_REORDER_HDR;
> > + g_free(value);
> > +
> > + value = svGetValue (ifcfg, "VLAN_FLAGS", FALSE);
> > + if (g_strstr_len(value, -1, "GVRP"))
> > + vlan_flags |= NM_VLAN_FLAG_GVRP;
> > + if (g_strstr_len(value, -1, "LOOSE_BINDING"))
> > + vlan_flags |= NM_VLAN_FLAG_LOOSE_BINDING;
> > +
> > + g_object_set(s_vlan, NM_SETTING_VLAN_FLAGS, vlan_flags, NULL);
> > + g_free(value);
> > +
> > + value = svGetValue (ifcfg, "VLAN_INGRESS_PRIORITY_MAP", FALSE);
> > + if (value) {
> > + gchar **list = NULL, **iter;
> > + list = g_strsplit_set (value, ",", 0);
> > + for (iter = list; iter && *iter; iter++) {
> > + if (**iter == '\0')
> > + continue;
>
> Instead of building up a GSList here, if you like you could add a
> nm_setting_add_priority_str() function to nm-setting-vlan.c that takes a
> const char* instead of 'a' and 'b' and does the string->uint conversion
> itself. Would save some code here.
>
> Dan
>
> > +
> > + if (!g_strrstr(*iter, ":"))
> > + continue;
> > +
> > + vlan_priority_ingress_map = g_slist_prepend (vlan_priority_ingress_map, *iter);
> > + }
> > + if (vlan_priority_ingress_map) {
> > + vlan_priority_ingress_map = g_slist_reverse (vlan_priority_ingress_map);
> > + g_object_set (s_vlan, NM_SETTING_VLAN_INGRESS_PRIORITY_MAP,
> > + vlan_priority_ingress_map, NULL);
> > + g_slist_free (vlan_priority_ingress_map);
> > + }
> > + g_free (value);
> > + g_strfreev (list);
> > + }
> > +
> > + value = svGetValue (ifcfg, "VLAN_EGRESS_PRIORITY_MAP", FALSE);
> > + if (value) {
> > + gchar **list = NULL, **iter;
> > + list = g_strsplit_set (value, ",", 0);
> > + for (iter = list; iter && *iter; iter++) {
> > + if (**iter == '\0')
> > + continue;
> > +
> > + if (!g_strrstr(*iter, ":"))
> > + continue;
> > +
> > + vlan_priority_egress_map = g_slist_prepend (vlan_priority_egress_map, *iter);
> > + }
> > + if (vlan_priority_egress_map) {
> > + vlan_priority_egress_map = g_slist_reverse (vlan_priority_egress_map);
> > + g_object_set (s_vlan, NM_SETTING_VLAN_EGRESS_PRIORITY_MAP,
> > + vlan_priority_egress_map, NULL);
> > + g_slist_free (vlan_priority_egress_map);
> > + }
> > + g_free (value);
> > + g_strfreev (list);
> > + }
> > +
> > + return (NMSetting *) s_vlan;
> > +
> > +free_interface_name:
> > + g_free(interface_name);
> > +error_return:
> > + g_object_unref(s_vlan);
> > + return NULL;
> > +}
> > +
> > +static NMConnection *
> > +vlan_connection_from_ifcfg (const char *file,
> > + shvarFile *ifcfg,
> > + gboolean nm_controlled,
> > + char **unmanaged,
> > + GError **error)
> > +{
> > + NMConnection *connection = NULL;
> > + NMSetting *con_setting = NULL;
> > + NMSetting *wired_setting = NULL;
> > + NMSetting *vlan_setting = NULL;
> > + NMSetting8021x *s_8021x = NULL;
> > +
> > + g_return_val_if_fail (file != NULL, NULL);
> > + g_return_val_if_fail (ifcfg != NULL, NULL);
> > +
> > + connection = nm_connection_new ();
> > + if (!connection) {
> > + g_set_error (error, IFCFG_PLUGIN_ERROR, 0,
> > + "Failed to allocate new connection for %s.", file);
> > + return NULL;
> > + }
> > +
> > + con_setting = make_connection_setting (file, ifcfg, NM_SETTING_VLAN_SETTING_NAME, NULL);
> > + if (!con_setting) {
> > + g_set_error (error, IFCFG_PLUGIN_ERROR, 0,
> > + "Failed to create connection setting.");
> > + g_object_unref (connection);
> > + return NULL;
> > + }
> > + nm_connection_add_setting (connection, con_setting);
> > +
> > + vlan_setting = make_vlan_setting (ifcfg, file, nm_controlled, unmanaged, &s_8021x, error);
> > + if (!vlan_setting) {
> > + g_object_unref (connection);
> > + return NULL;
> > + }
> > + nm_connection_add_setting (connection, vlan_setting);
> > +
> > + wired_setting = make_wired_setting (ifcfg, file, nm_controlled, unmanaged, &s_8021x, error);
> > + if (!wired_setting) {
> > + g_object_unref (connection);
> > + return NULL;
> > + }
> > + nm_connection_add_setting (connection, wired_setting);
> > +
> > + if (s_8021x)
> > + nm_connection_add_setting (connection, NM_SETTING (s_8021x));
> > + if (!nm_connection_verify (connection, error)) {
> > + g_object_unref (connection);
> > + return NULL;
> > + }
> > +
> > + return connection;
> > +}
> > +
> > static gboolean
> > is_wireless_device (const char *iface)
> > {
> > @@ -3510,7 +3685,6 @@ is_bond_device (const char *name, shvarFile *parsed)
> > enum {
> > IGNORE_REASON_NONE = 0x00,
> > IGNORE_REASON_BRIDGE = 0x01,
> > - IGNORE_REASON_VLAN = 0x02,
> > };
> >
> > NMConnection *
> > @@ -3630,7 +3804,7 @@ connection_from_file (const char *filename,
> > goto done;
> > }
> >
> > - /* Ignore BRIDGE= and VLAN= connections for now too (rh #619863) */
> > + /* Ignore BRIDGE= connections for now too (rh #619863) */
> > tmp = svGetValue (parsed, "BRIDGE", FALSE);
> > if (tmp) {
> > g_free (tmp);
> > @@ -3638,25 +3812,18 @@ connection_from_file (const char *filename,
> > ignore_reason = IGNORE_REASON_BRIDGE;
> > }
> >
> > - if (nm_controlled) {
> > - tmp = svGetValue (parsed, "VLAN", FALSE);
> > - if (tmp) {
> > - g_free (tmp);
> > - nm_controlled = FALSE;
> > - ignore_reason = IGNORE_REASON_VLAN;
> > - }
> > - }
> > -
> > /* Construct the connection */
> > if (!strcasecmp (type, TYPE_ETHERNET))
> > connection = wired_connection_from_ifcfg (filename, parsed, nm_controlled, unmanaged, &error);
> > else if (!strcasecmp (type, TYPE_WIRELESS))
> > connection = wireless_connection_from_ifcfg (filename, parsed, nm_controlled, unmanaged, &error);
> > - else if (!strcasecmp (type, TYPE_BRIDGE)) {
> > + else if (!strcasecmp (type, TYPE_BRIDGE))
> > g_set_error (&error, IFCFG_PLUGIN_ERROR, 0,
> > "Bridge connections are not yet supported");
> > - } else if (!strcasecmp (type, TYPE_BOND))
> > + else if (!strcasecmp (type, TYPE_BOND))
> > connection = bond_connection_from_ifcfg (filename, parsed, nm_controlled, unmanaged, &error);
> > + else if (!strcasecmp (type, TYPE_VLAN))
> > + connection = vlan_connection_from_ifcfg (filename, parsed, nm_controlled, unmanaged, &error);
> > else {
> > g_set_error (&error, IFCFG_PLUGIN_ERROR, 0,
> > "Unknown connection type '%s'", type);
> > diff --git a/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-interface b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-interface
> > index 6c84185..d6d3f99 100644
> > --- a/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-interface
> > +++ b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-interface
> > @@ -1,7 +1,12 @@
> > -DEVICE=eth1.43
> > VLAN=yes
> > +TYPE=vlan
> > +DEVICE=vlan43
> > +PHYSDEV=eth9
> > +REORDER_HDR=0
> > +VLAN_FLAGS=GVRP,LOOSE_BINDING
> > +VLAN_INGRESS_PRIORITY_MAP=0:1,2:5
> > +VLAN_EGRESS_PRIORITY_MAP=12:3,14:7
> > ONBOOT=yes
> > -BOOTPROTO=none
> > +BOOTPROTO=static
> > IPADDR=192.168.43.149
> > NETMASK=255.255.255.0
> > -
> > diff --git a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
> > index e32266c..4b84a59 100644
> > --- a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
> > +++ b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
> > @@ -11765,9 +11765,10 @@ test_read_vlan_interface (void)
> > &route6file,
> > &error,
> > &ignore_error);
> > - ASSERT (connection == NULL,
> > + ASSERT (connection != NULL,
> > "vlan-interface-read", "unexpected success reading %s", TEST_IFCFG_VLAN_INTERFACE);
> >
> > + nm_connection_dump(connection);
> > g_free (unmanaged);
> > g_free (keyfile);
> > g_free (routefile);
>
>
> _______________________________________________
> networkmanager-list mailing list
> networkmanager-list gnome org
> http://mail.gnome.org/mailman/listinfo/networkmanager-list
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
