Wed, Nov 06, 2013 at 01:17:38PM CET, [email protected] wrote:
>On Wed, 2013-09-25 at 15:37 +0200, Jiri Pirko wrote:
>> So far, we have 2 properties for interface name, connection.interface-name
>> and bond/bridge/team.interface-name. If user wants to change it, he need to
>> change both. This patchset adjusts the bond/bridge/team property so
>> it is the same as the connection one.
>> 
>> Jiri Pirko (4):
>>   nm-setting: adjust virtual interface name according to connection
>>     interface name
>>   nm-setting-bond: implement set_virtual_iface_name()
>>   nm-setting-bridge: implement set_virtual_iface_name()
>>   nm-setting-team: implement set_virtual_iface_name()
>> 
>>  libnm-util/nm-setting-bond.c       | 23 ++++++++++++++++++++++-
>>  libnm-util/nm-setting-bridge.c     | 24 ++++++++++++++++++++++--
>>  libnm-util/nm-setting-connection.c |  3 ++-
>>  libnm-util/nm-setting-team.c       | 23 ++++++++++++++++++++++-
>>  libnm-util/nm-setting.c            | 21 +++++++++++++++++++++
>>  libnm-util/nm-setting.h            |  2 ++
>>  6 files changed, 91 insertions(+), 5 deletions(-)
>> 
>
>
>Hi Jiri,
>
>
>I think in the first patch, nm_setting_set_virtual_iface_name should
>have "Since: 0.9.10" in the documentation.
>
>I find it a bit unexpected, that the verify() method actually changes
>the settings. Isn't there be a better place to fixup the setting?

IIRC this is the only place this can be done. Do you have any other in
mind?

>
>
>And in the following 3 patches, I think set_virtual_iface_name should
>return FALSE, if it did not change anything.
>So basically I would add 
>
>  if (!g_strcmp0 (priv->interface_name, iface_name))
>    return FALSE;
>
>
>Also, I would replace set_interface_name() completely with
>set_virtual_iface_name(), because they are only internal, static
>functions.
>
>
>
>Thomas


_______________________________________________
networkmanager-list mailing list
[email protected]
https://mail.gnome.org/mailman/listinfo/networkmanager-list

Reply via email to