Re: [PATCH 1/9] bonding: settings parser for ifcfg plugin + NMSettingBond class

[Thomas Graf <tgraf redhat com>]

On Tue, 2011-10-11 at 08:21 -0500, Dan Williams wrote:
> On Fri, 2011-09-23 at 14:52 +0200, Thomas Graf wrote:

> > +
> > +	if (strlen(priv->device_name) >= 16 ||
> > +	    !strcmp(priv->device_name, ".") ||
> > +	    !strcmp(priv->device_name, "..")) {
> 
> Do we want to check isalnum() here for every character?  What are the
> valid characters for a kernel device name?

The kernel uses the following function:

int dev_valid_name(const char *name)
{
	if (*name == '\0')
		return 0;
	if (strlen(name) >= IFNAMSIZ)
		return 0;
	if (!strcmp(name, ".") || !strcmp(name, ".."))
		return 0;

	while (*name) {
		if (*name == '/' || isspace(*name))
			return 0;
		name++;
	}
	return 1;
}

I'll copy and use that function.


> > +
> > +	/* XXX: Validate arp-ip-target */
> 
> Does this just require inet_pton() or something?

Depending on whether we are going to use sysfs we will need to provide
a numeric ip string. Also, currently only IPv4 is supported. Working
on figuring this out because IPv6 support is being worked on.

> > +	/**
> > +	 * NMSettingBond:device-name:
> > +	 *
> > +	 * Name of virtual kernel device
> > +	 **/
> > +	g_object_class_install_property
> > +		(object_class, PROP_DEVICE_NAME,
> 
> We could use IFACE_NAME or INTERFACE_NAME or such here; NM's usage of
> "device" is an anachronism from long ago which we don't really need to
> propagate if we don't want to.  Up to you.

Agreed, I will rename it.