3:25 p.m.
From: Ondrej Lichtner <olichtne(a)redhat.com>
The 'interface-name' value in device specific dictionaries was moved
into the general 'connection' dict. The old attributes are still
supported by NM (for compatibility) but since we don't use older NM
versions anymore I'm moving these to their proper place.
Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com>
---
lnst/Slave/NmConfigDevice.py | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/lnst/Slave/NmConfigDevice.py b/lnst/Slave/NmConfigDevice.py
index 8f9170e..29c2c75 100644
--- a/lnst/Slave/NmConfigDevice.py
+++ b/lnst/Slave/NmConfigDevice.py
@@ -494,6 +494,7 @@ class NmConfigDeviceBond(NmConfigDeviceGeneric):
s_bond_con = dbus.Dictionary({
'type': 'bond',
+ 'interface-name': config["name"],
'autoconnect': dbus.Boolean(False),
'uuid': config["master_uuid"],
'id': config["name"]+"_con"})
@@ -502,12 +503,9 @@ class NmConfigDeviceBond(NmConfigDeviceGeneric):
if options:
s_bond = dbus.Dictionary({
- 'interface-name': config["name"],
'options': options})
else:
- s_bond = dbus.Dictionary({
- 'interface-name': config["name"]})
-
+ s_bond = dbus.Dictionary({})
connection = dbus.Dictionary({
'bond': s_bond,
@@ -591,11 +589,11 @@ class NmConfigDeviceBridge(NmConfigDeviceGeneric):
s_bridge_con = dbus.Dictionary({
'type': 'bridge',
'autoconnect': dbus.Boolean(False),
+ 'interface-name': config["name"],
'uuid': config["master_uuid"],
'id': config["name"]+"_con"})
s_bridge = dbus.Dictionary({
- 'interface-name': config["name"],
'stp': dbus.Boolean(False)})
connection = dbus.Dictionary({
@@ -715,11 +713,11 @@ class NmConfigDeviceVlan(NmConfigDeviceGeneric):
s_vlan_con = dbus.Dictionary({
'type': 'vlan',
'autoconnect': dbus.Boolean(False),
+ 'interface-name': dev_name,
'uuid': str(uuid.uuid4()),
'id': dev_name+"_con"})
s_vlan = dbus.Dictionary({
- 'interface-name': dev_name,
'parent': realdev,
'id': dbus.UInt32(vlan_tci)}, signature="sv")
@@ -796,13 +794,13 @@ class NmConfigDeviceTeam(NmConfigDeviceGeneric):
s_team_con = dbus.Dictionary({
'type': 'team',
'autoconnect': dbus.Boolean(False),
+ 'interface-name': config["name"],
'uuid': config["master_uuid"],
'id': config["name"]+"_con"})
teamd_config = get_option(config, "teamd_config")
s_team = dbus.Dictionary({
- 'interface-name': config["name"],
'config': teamd_config})
connection = dbus.Dictionary({
--
2.10.0
3:25 p.m.
New subject: [PATCH 2/3] NetTestSlave: fix SlaveMethods return values
From: Ondrej Lichtner <olichtne(a)redhat.com>
Due to how the Ctl-Slave communication currently works, returning None
from a Slave Method is not possible - the main Slave loop doesn't resend
this to the Ctl, resulting in the Ctl waiting forever. This is due to
how background and foreground commands work - background command
immediately returns "passed" (meaning started) whereas a forground
command at first returns None, the command then runs in a forked process
and then returns the command results - meanwhile the Ctl is waiting
because it didn't get the "return None" message.
So returning None is fine as long as a different result message is
generated and returned later, currently that only happens in the
run_command method and the others are now fixed by this commit.
Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com>
---
lnst/Slave/NetTestSlave.py | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/lnst/Slave/NetTestSlave.py b/lnst/Slave/NetTestSlave.py
index da1e2f4..f6976db 100644
--- a/lnst/Slave/NetTestSlave.py
+++ b/lnst/Slave/NetTestSlave.py
@@ -147,7 +147,7 @@ class SlaveMethods:
if device:
return device.get_if_data()
else:
- return None
+ return {}
def get_devices_by_devname(self, devname):
name_scan = self._if_manager.get_devices()
@@ -190,14 +190,14 @@ class SlaveMethods:
def get_if_data(self, if_id):
dev = self._if_manager.get_mapped_device(if_id)
if dev is None:
- return None
+ return {}
return dev.get_if_data()
def link_stats(self, if_id):
dev = self._if_manager.get_mapped_device(if_id)
if dev is None:
logging.error("Device with id '%s' not found." % if_id)
- return None
+ return {}
return dev.link_stats()
def set_addresses(self, if_id, ips):
@@ -670,7 +670,7 @@ class SlaveMethods:
self._net_namespaces[netns] = {"pid": pid,
"pipe": read_pipe}
self._server_handler.add_netns(netns, read_pipe)
- return None
+ return False
elif pid == 0:
self._slave_server.set_netns_sighandlers()
#create new network namespace
--
2.10.0
3:25 p.m.
New subject: [PATCH 3/3] Slave: deconfigure only created interfaces
From: Ondrej Lichtner <olichtne(a)redhat.com>
Sometimes when configuration of the interface failed during it's
creation (and the device wasn't created, e.g. missing team{d, ctl}). The
slave would register this as a "configured" Device (because it had a
{Net, Nm}ConfigDevice object created, and would attempt to deconfigure
it, this return an exception again and either crash the slave or leave
it in an inconsistent state (trying to deconfigure the device again and
again).
This makes sure that deconfiguration is skipped in case LNST didn't
receive a netlink notification that the device was created and
registered by the kernel.
Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com>
---
lnst/Slave/InterfaceManager.py | 13 +++++++------
lnst/Slave/NetTestSlave.py | 5 +++--
2 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/lnst/Slave/InterfaceManager.py b/lnst/Slave/InterfaceManager.py
index 949817c..dc390a9 100644
--- a/lnst/Slave/InterfaceManager.py
+++ b/lnst/Slave/InterfaceManager.py
@@ -566,12 +566,13 @@ class Device(object):
m_dev.clear_configuration()
if self._conf != None:
- self.address_cleanup()
- self.down()
- self.deconfigure()
- self._clear_tc_qdisc()
- self._clear_tc_filters()
- self.destroy()
+ if self._if_index is not None:
+ self.address_cleanup()
+ self.down()
+ self.deconfigure()
+ self._clear_tc_qdisc()
+ self._clear_tc_filters()
+ self.destroy()
self._conf = None
self._conf_dict = None
diff --git a/lnst/Slave/NetTestSlave.py b/lnst/Slave/NetTestSlave.py
index f6976db..2d0b361 100644
--- a/lnst/Slave/NetTestSlave.py
+++ b/lnst/Slave/NetTestSlave.py
@@ -383,7 +383,7 @@ class SlaveMethods:
def deconfigure_interface(self, if_id):
device = self._if_manager.get_mapped_device(if_id)
- if device is not None:
+ if device is not None and device.get_if_index() is not None:
device.clear_configuration()
else:
logging.error("No device with id '%s' to deconfigure." %
if_id)
@@ -533,7 +533,8 @@ class SlaveMethods:
for if_id, dev in devs.iteritems():
peer = dev.get_peer()
if peer == None:
- dev.clear_configuration()
+ if dev.get_if_index() is not None:
+ dev.clear_configuration()
else:
peer_if_index = peer.get_if_index()
peer_id = self._if_manager.get_id_by_if_index(peer_if_index)
--
2.10.0
2:17 a.m.
New subject: [PATCH 3/3] Slave: deconfigure only created interfaces
Thu, Sep 22, 2016 at 10:25:24PM CEST, olichtne(a)redhat.com wrote:
From: Ondrej Lichtner <olichtne(a)redhat.com>
Sometimes when configuration of the interface failed during it's
creation (and the device wasn't created, e.g. missing team{d, ctl}). The
slave would register this as a "configured" Device (because it had a
{Net, Nm}ConfigDevice object created, and would attempt to deconfigure
it, this return an exception again and either crash the slave or leave
it in an inconsistent state (trying to deconfigure the device again and
again).
This makes sure that deconfiguration is skipped in case LNST didn't
receive a netlink notification that the device was created and
registered by the kernel.
Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com>
---
lnst/Slave/InterfaceManager.py | 13 +++++++------
lnst/Slave/NetTestSlave.py | 5 +++--
2 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/lnst/Slave/InterfaceManager.py b/lnst/Slave/InterfaceManager.py
index 949817c..dc390a9 100644
--- a/lnst/Slave/InterfaceManager.py
+++ b/lnst/Slave/InterfaceManager.py
@@ -566,12 +566,13 @@ class Device(object):
m_dev.clear_configuration()
if self._conf != None:
- self.address_cleanup()
- self.down()
- self.deconfigure()
- self._clear_tc_qdisc()
- self._clear_tc_filters()
- self.destroy()
+ if self._if_index is not None:
+ self.address_cleanup()
+ self.down()
+ self.deconfigure()
+ self._clear_tc_qdisc()
+ self._clear_tc_filters()
+ self.destroy()
self._conf = None
self._conf_dict = None
diff --git a/lnst/Slave/NetTestSlave.py b/lnst/Slave/NetTestSlave.py
index f6976db..2d0b361 100644
--- a/lnst/Slave/NetTestSlave.py
+++ b/lnst/Slave/NetTestSlave.py
@@ -383,7 +383,7 @@ class SlaveMethods:
def deconfigure_interface(self, if_id):
device = self._if_manager.get_mapped_device(if_id)
- if device is not None:
+ if device is not None and device.get_if_index() is not None:
^^^^ Is this check necessary? You
already do the check in the hunk above.
> device.clear_configuration()
> else:
> logging.error("No device with id '%s' to deconfigure."
% if_id)
>@@ -533,7 +533,8 @@ class SlaveMethods:
> for if_id, dev in devs.iteritems():
> peer = dev.get_peer()
> if peer == None:
>- dev.clear_configuration()
>+ if dev.get_if_index() is not None:
>+ dev.clear_configuration()
> else:
> peer_if_index = peer.get_if_index()
> peer_id = self._if_manager.get_id_by_if_index(peer_if_index)
>--
>2.10.0
>_______________________________________________
>LNST-developers mailing list -- lnst-developers(a)lists.fedorahosted.org
>To unsubscribe send an email to lnst-developers-leave(a)lists.fedorahosted.org
2:41 a.m.
New subject: [PATCH 3/3] Slave: deconfigure only created interfaces
On Fri, Sep 23, 2016 at 09:17:13AM +0200, Jan Tluka wrote:
Thu, Sep 22, 2016 at 10:25:24PM CEST, olichtne(a)redhat.com wrote:
>From: Ondrej Lichtner <olichtne(a)redhat.com>
>
>Sometimes when configuration of the interface failed during it's
>creation (and the device wasn't created, e.g. missing team{d, ctl}). The
>slave would register this as a "configured" Device (because it had a
>{Net, Nm}ConfigDevice object created, and would attempt to deconfigure
>it, this return an exception again and either crash the slave or leave
>it in an inconsistent state (trying to deconfigure the device again and
>again).
>
>This makes sure that deconfiguration is skipped in case LNST didn't
>receive a netlink notification that the device was created and
>registered by the kernel.
>
>Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com>
>---
> lnst/Slave/InterfaceManager.py | 13 +++++++------
> lnst/Slave/NetTestSlave.py | 5 +++--
> 2 files changed, 10 insertions(+), 8 deletions(-)
>
>diff --git a/lnst/Slave/InterfaceManager.py b/lnst/Slave/InterfaceManager.py
>index 949817c..dc390a9 100644
>--- a/lnst/Slave/InterfaceManager.py
>+++ b/lnst/Slave/InterfaceManager.py
>@@ -566,12 +566,13 @@ class Device(object):
> m_dev.clear_configuration()
>
> if self._conf != None:
>- self.address_cleanup()
>- self.down()
>- self.deconfigure()
>- self._clear_tc_qdisc()
>- self._clear_tc_filters()
>- self.destroy()
>+ if self._if_index is not None:
>+ self.address_cleanup()
>+ self.down()
>+ self.deconfigure()
>+ self._clear_tc_qdisc()
>+ self._clear_tc_filters()
>+ self.destroy()
> self._conf = None
> self._conf_dict = None
>
>diff --git a/lnst/Slave/NetTestSlave.py b/lnst/Slave/NetTestSlave.py
>index f6976db..2d0b361 100644
>--- a/lnst/Slave/NetTestSlave.py
>+++ b/lnst/Slave/NetTestSlave.py
>@@ -383,7 +383,7 @@ class SlaveMethods:
>
> def deconfigure_interface(self, if_id):
> device = self._if_manager.get_mapped_device(if_id)
>- if device is not None:
>+ if device is not None and device.get_if_index() is not None:
^^^^ Is this check necessary? You
already do the check in the hunk above.
You're right, it's not necessary, and I also just noticed that the
Device class has an attribute _configured that is set to True if
configuration succeeds so it can probably be used more reliably than
waiting for a netlink notification (wrt. race conditions).
The _configured flag is already used by the deconfigure() method, the
problems were caused by the other methods, so I'll send a v2 of the
patch that extends the use of the flag to all the other methods.
Thanks for catching this.
-Ondrej
10:58 a.m.
New subject: [PATCH 1/3] NmConfigDevice: move name values to 'connection'
dicts
On Thu, Sep 22, 2016 at 10:25:22PM +0200, olichtne(a)redhat.com wrote:
From: Ondrej Lichtner <olichtne(a)redhat.com>
The 'interface-name' value in device specific dictionaries was moved
into the general 'connection' dict. The old attributes are still
supported by NM (for compatibility) but since we don't use older NM
versions anymore I'm moving these to their proper place.
Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com>
---
lnst/Slave/NmConfigDevice.py | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/lnst/Slave/NmConfigDevice.py b/lnst/Slave/NmConfigDevice.py
index 8f9170e..29c2c75 100644
--- a/lnst/Slave/NmConfigDevice.py
+++ b/lnst/Slave/NmConfigDevice.py
@@ -494,6 +494,7 @@ class NmConfigDeviceBond(NmConfigDeviceGeneric):
s_bond_con = dbus.Dictionary({
'type': 'bond',
+ 'interface-name': config["name"],
'autoconnect': dbus.Boolean(False),
'uuid': config["master_uuid"],
'id': config["name"]+"_con"})
@@ -502,12 +503,9 @@ class NmConfigDeviceBond(NmConfigDeviceGeneric):
if options:
s_bond = dbus.Dictionary({
- 'interface-name': config["name"],
'options': options})
else:
- s_bond = dbus.Dictionary({
- 'interface-name': config["name"]})
-
+ s_bond = dbus.Dictionary({})
connection = dbus.Dictionary({
'bond': s_bond,
@@ -591,11 +589,11 @@ class NmConfigDeviceBridge(NmConfigDeviceGeneric):
s_bridge_con = dbus.Dictionary({
'type': 'bridge',
'autoconnect': dbus.Boolean(False),
+ 'interface-name': config["name"],
'uuid': config["master_uuid"],
'id': config["name"]+"_con"})
s_bridge = dbus.Dictionary({
- 'interface-name': config["name"],
'stp': dbus.Boolean(False)})
connection = dbus.Dictionary({
@@ -715,11 +713,11 @@ class NmConfigDeviceVlan(NmConfigDeviceGeneric):
s_vlan_con = dbus.Dictionary({
'type': 'vlan',
'autoconnect': dbus.Boolean(False),
+ 'interface-name': dev_name,
'uuid': str(uuid.uuid4()),
'id': dev_name+"_con"})
s_vlan = dbus.Dictionary({
- 'interface-name': dev_name,
'parent': realdev,
'id': dbus.UInt32(vlan_tci)}, signature="sv")
@@ -796,13 +794,13 @@ class NmConfigDeviceTeam(NmConfigDeviceGeneric):
s_team_con = dbus.Dictionary({
'type': 'team',
'autoconnect': dbus.Boolean(False),
+ 'interface-name': config["name"],
'uuid': config["master_uuid"],
'id': config["name"]+"_con"})
teamd_config = get_option(config, "teamd_config")
s_team = dbus.Dictionary({
- 'interface-name': config["name"],
'config': teamd_config})
connection = dbus.Dictionary({
--
2.10.0
scratch patchset, I sent a v2 that adds a new patch and fixes 3/3 based
on suggestion from jtluka
-Ondrej
2772
days inactive
2773
days old
lnst-developers@lists.fedorahosted.org
5 comments
2 participants
participants (2)
-
Jan Tluka -
Ondrej Lichtner