Change in vdsm[master]: net: Split OVS setup transaction to adjust for bond setup

[danken]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: net: Split OVS setup transaction to adjust for bond setup
..


net: Split OVS setup transaction to adjust for bond setup

When using Linux bonds (instead of OVS bonds), the OVS setup transaction
needs to be split into two.
First one for removing networks and a second for adding networks.
In between, the bonds setup is placed.

These three steps contain, in addition to the actual devices setup, the
acquire and running-config update, allowing proper rollback in case of
setup failure.

This patch, fixes a scenario that fails with the previous patch, where
the Linux bonds have been introduced in the setup flow.
The scenario: Move a network nic from a network to a bond slave.

Change-Id: I0bd351e755be9d5dcaf5a05e5b79345e6f7bcfe8
Signed-off-by: Edward Haas 
Reviewed-on: https://gerrit.ovirt.org/64118
Continuous-Integration: Jenkins CI
Reviewed-by: Petr Horáček 
Reviewed-by: Dan Kenigsberg 
---
M lib/vdsm/network/netswitch.py
M lib/vdsm/network/ovs/switch.py
M tests/network/ovs_switch_test.py
M tests/network/ovs_test.py
4 files changed, 111 insertions(+), 91 deletions(-)

Approvals:
  Jenkins CI: Passed CI tests
  Petr Horáček: Looks good to me, but someone else must approve
  Dan Kenigsberg: Looks good to me, approved
  Edward Haas: Verified



-- 
To view, visit https://gerrit.ovirt.org/64118
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I0bd351e755be9d5dcaf5a05e5b79345e6f7bcfe8
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: net: Split OVS setup transaction to adjust for bond setup

[automation]

gerrit-hooks has posted comments on this change.

Change subject: net: Split OVS setup transaction to adjust for bond setup
..


Patch Set 7:

* Update tracker: IGNORE, no Bug-Url found
* Set MODIFIED::IGNORE, no Bug-Url found.

-- 
To view, visit https://gerrit.ovirt.org/64118
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0bd351e755be9d5dcaf5a05e5b79345e6f7bcfe8
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: net: Split OVS setup transaction to adjust for bond setup

[danken]

Dan Kenigsberg has posted comments on this change.

Change subject: net: Split OVS setup transaction to adjust for bond setup
..


Patch Set 6: Code-Review+2

raising

-- 
To view, visit https://gerrit.ovirt.org/64118
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0bd351e755be9d5dcaf5a05e5b79345e6f7bcfe8
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: net: Split OVS setup transaction to adjust for bond setup

[phoracek]

Petr Horáček has posted comments on this change.

Change subject: net: Split OVS setup transaction to adjust for bond setup
..


Patch Set 6: Code-Review+1

-- 
To view, visit https://gerrit.ovirt.org/64118
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0bd351e755be9d5dcaf5a05e5b79345e6f7bcfe8
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: net: Split OVS setup transaction to adjust for bond setup

[edwardh]

Edward Haas has posted comments on this change.

Change subject: net: Split OVS setup transaction to adjust for bond setup
..


Patch Set 6: Verified+1

-- 
To view, visit https://gerrit.ovirt.org/64118
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0bd351e755be9d5dcaf5a05e5b79345e6f7bcfe8
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: net: Split OVS setup transaction to adjust for bond setup

[automation]

gerrit-hooks has posted comments on this change.

Change subject: net: Split OVS setup transaction to adjust for bond setup
..


Patch Set 6:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/64118
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0bd351e755be9d5dcaf5a05e5b79345e6f7bcfe8
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: net: Split OVS setup transaction to adjust for bond setup

[edwardh]

Edward Haas has posted comments on this change.

Change subject: net: Split OVS setup transaction to adjust for bond setup
..


Patch Set 4:

(1 comment)

https://gerrit.ovirt.org/#/c/64118/4/lib/vdsm/network/netswitch.py
File lib/vdsm/network/netswitch.py:

Line 202: 
Line 203: def _add_networks(nets2add, ovs_info, config, acq):
Line 204: net_add_setup = ovs_switch.create_network_addition_setup(ovs_info)
Line 205: net_add_setup.add(nets2add)
Line 206: acq.acquire(net_add_setup.acquired_ifaces)
> before we gathered ifaces to be acquired, acquired them and then transactio
OK, now I understand... You are right.
Line 207: for net, attrs in six.iteritems(nets2add):
Line 208: config.setNetwork(net, attrs)
Line 209: 
Line 210: 


-- 
To view, visit https://gerrit.ovirt.org/64118
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0bd351e755be9d5dcaf5a05e5b79345e6f7bcfe8
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: net: Split OVS setup transaction to adjust for bond setup

[automation]

gerrit-hooks has posted comments on this change.

Change subject: net: Split OVS setup transaction to adjust for bond setup
..


Patch Set 5:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/64118
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0bd351e755be9d5dcaf5a05e5b79345e6f7bcfe8
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: net: Split OVS setup transaction to adjust for bond setup

[phoracek]

Petr Horáček has posted comments on this change.

Change subject: net: Split OVS setup transaction to adjust for bond setup
..


Patch Set 4:

(1 comment)

https://gerrit.ovirt.org/#/c/64118/4/lib/vdsm/network/netswitch.py
File lib/vdsm/network/netswitch.py:

Line 202: 
Line 203: def _add_networks(nets2add, ovs_info, config, acq):
Line 204: net_add_setup = ovs_switch.create_network_addition_setup(ovs_info)
Line 205: net_add_setup.add(nets2add)
Line 206: acq.acquire(net_add_setup.acquired_ifaces)
> net_add_setup.add(nets2add) is done in a transaction, so we save it was suc
before we gathered ifaces to be acquired, acquired them and then transaction 
was commited. thanks to that, we marked ifaces as ours before system changes 
(no time for NM to break it)
Line 207: for net, attrs in six.iteritems(nets2add):
Line 208: config.setNetwork(net, attrs)
Line 209: 
Line 210: 


-- 
To view, visit https://gerrit.ovirt.org/64118
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0bd351e755be9d5dcaf5a05e5b79345e6f7bcfe8
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: net: Split OVS setup transaction to adjust for bond setup

[edwardh]

Edward Haas has posted comments on this change.

Change subject: net: Split OVS setup transaction to adjust for bond setup
..


Patch Set 4: Verified+1

Passing all unit and functional tests.

-- 
To view, visit https://gerrit.ovirt.org/64118
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0bd351e755be9d5dcaf5a05e5b79345e6f7bcfe8
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: net: Split OVS setup transaction to adjust for bond setup

[edwardh]

Edward Haas has posted comments on this change.

Change subject: net: Split OVS setup transaction to adjust for bond setup
..


Patch Set 4:

(3 comments)

https://gerrit.ovirt.org/#/c/64118/4/lib/vdsm/network/netswitch.py
File lib/vdsm/network/netswitch.py:

Line 202: 
Line 203: def _add_networks(nets2add, ovs_info, config, acq):
Line 204: net_add_setup = ovs_switch.create_network_addition_setup(ovs_info)
Line 205: net_add_setup.add(nets2add)
Line 206: acq.acquire(net_add_setup.acquired_ifaces)
> but we should acquire before we commit add(), right?
net_add_setup.add(nets2add) is done in a transaction, so we save it was 
successful.
(Like we did before)
Line 207: for net, attrs in six.iteritems(nets2add):
Line 208: config.setNetwork(net, attrs)
Line 209: 
Line 210: 


https://gerrit.ovirt.org/#/c/64118/4/lib/vdsm/network/ovs/switch.py
File lib/vdsm/network/ovs/switch.py:

Line 122: # FIXME: What about an existing bond?
Line 123: if nic is not None and vlan is None:
Line 124: self._copy_nic_hwaddr_to_nb(net, nic)
Line 125: 
Line 126: self._ovs_info.northbounds_by_sb.setdefault(sb, set())
> .add(net) won't  fit here?
Need a new line and it makes it less readable, so I preferred to split it.
Line 127: self._ovs_info.northbounds_by_sb[sb].add(net)
Line 128: 
Line 129: @property
Line 130: def acquired_ifaces(self):


Line 197: """
Line 198: return [ovsdb.del_br(bridge) for bridge in _unused_bridges()]
Line 199: 
Line 200: 
Line 201: # TODO: we can just check for bridges with no NB port
> can we address this? we detach both NICs and bonds from bridge when not use
I cleaned this up in the next patch, please have a look.
Line 202: def _unused_bridges():
Line 203: unused_bridges = set()
Line 204: ovs_info = info.OvsInfo()
Line 205: for bridge, attrs in six.iteritems(ovs_info.bridges):


-- 
To view, visit https://gerrit.ovirt.org/64118
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0bd351e755be9d5dcaf5a05e5b79345e6f7bcfe8
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: net: Split OVS setup transaction to adjust for bond setup

[phoracek]

Petr Horáček has posted comments on this change.

Change subject: net: Split OVS setup transaction to adjust for bond setup
..


Patch Set 4:

(3 comments)

https://gerrit.ovirt.org/#/c/64118/4/lib/vdsm/network/netswitch.py
File lib/vdsm/network/netswitch.py:

Line 202: 
Line 203: def _add_networks(nets2add, ovs_info, config, acq):
Line 204: net_add_setup = ovs_switch.create_network_addition_setup(ovs_info)
Line 205: net_add_setup.add(nets2add)
Line 206: acq.acquire(net_add_setup.acquired_ifaces)
but we should acquire before we commit add(), right?
Line 207: for net, attrs in six.iteritems(nets2add):
Line 208: config.setNetwork(net, attrs)
Line 209: 
Line 210: 


https://gerrit.ovirt.org/#/c/64118/4/lib/vdsm/network/ovs/switch.py
File lib/vdsm/network/ovs/switch.py:

Line 122: # FIXME: What about an existing bond?
Line 123: if nic is not None and vlan is None:
Line 124: self._copy_nic_hwaddr_to_nb(net, nic)
Line 125: 
Line 126: self._ovs_info.northbounds_by_sb.setdefault(sb, set())
.add(net) won't  fit here?
Line 127: self._ovs_info.northbounds_by_sb[sb].add(net)
Line 128: 
Line 129: @property
Line 130: def acquired_ifaces(self):


Line 197: """
Line 198: return [ovsdb.del_br(bridge) for bridge in _unused_bridges()]
Line 199: 
Line 200: 
Line 201: # TODO: we can just check for bridges with no NB port
can we address this? we detach both NICs and bonds from bridge when not used 
anymore.
Line 202: def _unused_bridges():
Line 203: unused_bridges = set()
Line 204: ovs_info = info.OvsInfo()
Line 205: for bridge, attrs in six.iteritems(ovs_info.bridges):


-- 
To view, visit https://gerrit.ovirt.org/64118
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0bd351e755be9d5dcaf5a05e5b79345e6f7bcfe8
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: net: Split OVS setup transaction to adjust for bond setup

[edwardh]

Edward Haas has posted comments on this change.

Change subject: net: Split OVS setup transaction to adjust for bond setup
..


Patch Set 3:

(2 comments)

https://gerrit.ovirt.org/#/c/64118/3//COMMIT_MSG
Commit Message:

PS3, Line 18: fixies
> fixes
Done


https://gerrit.ovirt.org/#/c/64118/3/lib/vdsm/network/netswitch.py
File lib/vdsm/network/netswitch.py:

PS3, Line 204: net_add_setup = 
ovs_switch.create_network_addition_setup(ovs_info)
 : net_add_setup.add(nets2add)
 : acq.acquire(net_add_setup.acquired_ifaces)
> do we need to keep setup class here? it would be easier to read if it was a
Initially I tried it in the same class (Setup), then decided to go with a 
separate class, dedicated for add (and another for remove).
Main reasoning was the multiple inner small methods, with multiple shared data. 
I prefer a class that can save the data for the methods to use it, over passing 
a large number of arguments between the methods. The later is just too messy 
for me.


-- 
To view, visit https://gerrit.ovirt.org/64118
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0bd351e755be9d5dcaf5a05e5b79345e6f7bcfe8
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: net: Split OVS setup transaction to adjust for bond setup

[automation]

gerrit-hooks has posted comments on this change.

Change subject: net: Split OVS setup transaction to adjust for bond setup
..


Patch Set 4:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/64118
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0bd351e755be9d5dcaf5a05e5b79345e6f7bcfe8
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: net: Split OVS setup transaction to adjust for bond setup

[phoracek]

Petr Horáček has posted comments on this change.

Change subject: net: Split OVS setup transaction to adjust for bond setup
..


Patch Set 3: Code-Review-1

(2 comments)

https://gerrit.ovirt.org/#/c/64118/3//COMMIT_MSG
Commit Message:

PS3, Line 18: fixies
fixes


https://gerrit.ovirt.org/#/c/64118/3/lib/vdsm/network/netswitch.py
File lib/vdsm/network/netswitch.py:

PS3, Line 204: net_add_setup = 
ovs_switch.create_network_addition_setup(ovs_info)
 : net_add_setup.add(nets2add)
 : acq.acquire(net_add_setup.acquired_ifaces)
do we need to keep setup class here? it would be easier to read if it was a 
plain function:

acquired_ifaces = ovs_switch.add_networks(networks, ovs_info)


-- 
To view, visit https://gerrit.ovirt.org/64118
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0bd351e755be9d5dcaf5a05e5b79345e6f7bcfe8
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: net: Split OVS setup transaction to adjust for bond setup

[automation]

gerrit-hooks has posted comments on this change.

Change subject: net: Split OVS setup transaction to adjust for bond setup
..


Patch Set 3:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/64118
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0bd351e755be9d5dcaf5a05e5b79345e6f7bcfe8
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: net: Split OVS setup transaction to adjust for bond setup

[automation]

gerrit-hooks has posted comments on this change.

Change subject: net: Split OVS setup transaction to adjust for bond setup
..


Patch Set 2:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/64118
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0bd351e755be9d5dcaf5a05e5b79345e6f7bcfe8
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: net: Split OVS setup transaction to adjust for bond setup

[automation]

gerrit-hooks has posted comments on this change.

Change subject: net: Split OVS setup transaction to adjust for bond setup
..


Patch Set 1:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/64118
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0bd351e755be9d5dcaf5a05e5b79345e6f7bcfe8
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: net: Split OVS setup transaction to adjust for bond setup

[edwardh]

Edward Haas has uploaded a new change for review.

Change subject: net: Split OVS setup transaction to adjust for bond setup
..

net: Split OVS setup transaction to adjust for bond setup

When using Linux bonds (instead of OVS bonds), the OVS setup transaction
needs to be split into two.
First one for removing networks and a second for adding networks.
In between, the bonds setup is placed.

These three steps contain, in addition to the actual devices setup, the
acquire and running-config update, allowing proper rollback in case of
setup failure.

This patch, fixies a scenario that fails with the previous patch, where
the Linux bonds have been introduced in the setup flow.
The scenario: Move a network nic from a network to a bond slave.

Change-Id: I0bd351e755be9d5dcaf5a05e5b79345e6f7bcfe8
Signed-off-by: Edward Haas 
---
M lib/vdsm/network/netswitch.py
M lib/vdsm/network/ovs/switch.py
M tests/network/ovs_switch_test.py
M tests/network/ovs_test.py
4 files changed, 106 insertions(+), 89 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/18/64118/1

diff --git a/lib/vdsm/network/netswitch.py b/lib/vdsm/network/netswitch.py
index 70d2cd6..529957a 100644
--- a/lib/vdsm/network/netswitch.py
+++ b/lib/vdsm/network/netswitch.py
@@ -172,37 +172,40 @@
 
 # FIXME: we are not able to move a nic from bond to network in one setup
 with Transaction(in_rollback=in_rollback) as config:
-setup_bonds = SetupBonds(bonds2add, bonds2edit, bonds2remove, config)
 with ifacquire.Transaction(ovs_netinfo['networks']) as acq:
-with ovs_switch.create_setup(_ovs_info) as setup_ovs:
-setup_ovs.remove_nets(nets2remove)
-setup_bonds.remove_bonds()
-acq.acquire(setup_bonds.ifaces_for_acquirement)
-setup_bonds.edit_bonds()
-setup_bonds.add_bonds()
-setup_ovs.add_nets(nets2add)
-acq.acquire(setup_ovs.acquired_ifaces)
-_update_networks_running_config(networks, config)
+_remove_networks(nets2remove, _ovs_info, config)
+_setup_bonds(bonds2add, bonds2edit, bonds2remove, config, acq)
+_add_networks(nets2add, _ovs_info, config, acq)
+
 ovs_switch.cleanup()
 setup_ipv6autoconf(networks)
 set_ovs_links_up(nets2add, bonds2add, bonds2edit)
 setup_ovs_ip_config(nets2add, nets2remove)
+
 connectivity.check(options)
 
 
-# TODO: We should use KernelConfig when it will be fully reliable.
-def _update_networks_running_config(networks, running_config):
-"""
-Update running_config with the networks configuration.
+def _remove_networks(nets2remove, ovs_info, config):
+net_rem_setup = ovs_switch.create_network_removal_setup(ovs_info)
+net_rem_setup.remove(nets2remove)
+for net, attrs in six.iteritems(nets2remove):
+config.removeNetwork(net)
 
-This step has to be done as soon as we apply the changes in the system.
-The running config will be used to generate the rollback query.
-"""
-for net, attrs in six.iteritems(networks):
-if 'remove' in attrs:
-running_config.removeNetwork(net)
-else:
-running_config.setNetwork(net, attrs)
+
+def _setup_bonds(bonds2add, bonds2edit, bonds2remove, config, acq):
+setup_bonds = SetupBonds(bonds2add, bonds2edit, bonds2remove, config)
+setup_bonds.remove_bonds()
+acq.acquire(setup_bonds.ifaces_for_acquirement)
+setup_bonds.edit_bonds()
+setup_bonds.add_bonds()
+
+
+def _add_networks(nets2add, ovs_info, config, acq):
+net_add_setup = ovs_switch.create_network_addition_setup(ovs_info)
+net_add_setup.add(nets2add)
+acq.acquire(net_add_setup.acquired_ifaces)
+for net, attrs in six.iteritems(nets2add):
+config.setNetwork(net, attrs)
 
 
 def setup_ovs_ip_config(nets2add, nets2remove):
diff --git a/lib/vdsm/network/ovs/switch.py b/lib/vdsm/network/ovs/switch.py
index 1b47315..1e16a88 100644
--- a/lib/vdsm/network/ovs/switch.py
+++ b/lib/vdsm/network/ovs/switch.py
@@ -53,84 +53,96 @@
 t.add(*_cleanup_unused_bridges(ovsdb))
 
 
-def create_setup(ovs_info):
+def create_network_removal_setup(ovs_info):
 ovsdb = driver.create()
-return Setup(ovsdb, ovs_info)
+return NetsRemovalSetup(ovsdb, ovs_info)
 
 
-# TODO: We could move all setup() code into __init__ and __exit__.
-class Setup(object):
+def create_network_addition_setup(ovs_info):
+ovsdb = driver.create()
+return NetsAdditionSetup(ovsdb, ovs_info)
+
+
+class NetsRemovalSetup(object):
 def __init__(self, ovsdb, ovs_info):
 self._ovsdb = ovsdb
-self._transaction = self._ovsdb.transaction()
 self._ovs_info = ovs_info
-self._bridges_by_sb = ovs_info.bridges_by_sb
-self._northbounds_by_sb = ovs_info.northbounds_by_sb
+