Re: [PATCH] bonding: Writer support

[Thomas Graf <tgraf redhat com>]

On Fri, 2012-02-24 at 13:59 +0100, Jirka Klimes wrote:
> On Tuesday 21 of February 2012 13:58:55 Thomas Graf wrote:
> > For bonding-master:
> >   TYPE=bond
> >   BONDING_MASTER=yes
> >   DEVICE=<NAME>
> >   BONDING_OPTS="..."
> > 
> > For bonding-slaves:
> >   MASTER=<NAME>
> > 
> > The test cases currently fail for reasons I don't understand. When
> > dumping, both connection and reread look identical.
> > 
> > Signed-off-by: Thomas Graf <tgraf redhat com>
> > ---
> 
> Looks good in general.
> 
> The problem is that in you didn't save wired setting (802-3-ethernet)
> in test_write_bond_main(), but while re-reading, it is created.

I see, so non-existing and empty is not considered a match.

> Should wired setting be present in both master and slave connections?

Yes, it's used to set the MTU and eventually the mac address.

> > +	              NULL);
> > +	g_free (uuid);
> > +
>         /* Wired setting */
>         s_wired = (NMSettingWired *) nm_setting_wired_new ();
>         ASSERT (s_wired != NULL,
>                 "bond-main-write", "failed to allocate new %s setting",
>                 NM_SETTING_WIRED_SETTING_NAME);
>         nm_connection_add_setting (connection, NM_SETTING (s_wired));

OK. This makes the bond-main test succeed.

> > +	/* bond setting */
> > +	s_bond = (NMSettingBond *) nm_setting_bond_new ();
> > +	ASSERT (s_bond != NULL,
> 
> 
> Another issue is in test_write_bond_slave():
> In src/settings/plugins/ifcfg-rh/writer.c:1424 we return, when IPv4 method is 
> disabled (NM_SETTING_IP4_CONFIG_METHOD_DISABLED) and that's why
> MAY_FAIL you set to TRUE is not written. And while re-reading MAY_FAIL
> become FALSE (the default value).
> 
> g_object_set (s_ip4,
>               NM_SETTING_IP4_CONFIG_METHOD,
>               NM_SETTING_IP4_CONFIG_METHOD_DISABLED,             
> 	      NULL);

OK. This explains it. I've removed setting MAY_FAIL to resolve the test.

> I am not sure if we should store other values when the method is
> disabled. As in that case we are not interested in the values, I guess. Dan,
> what do you think?

I think it can be done either way. I just didn't think of this.

Thomas