Edward Haas has posted comments on this change. Change subject: net: Setup validation for OVS - Check nics usage ......................................................................
Patch Set 4: (3 comments) https://gerrit.ovirt.org/#/c/64212/4/lib/vdsm/network/netswitch.py File lib/vdsm/network/netswitch.py: PS4, Line 182: ovs_nets = ovs_info.create_netinfo(_ovs_info)['networks'] : ovs_switch.validator.validate_nic_usage( : nets2add, bonds2add, : _get_kernel_nets_nics(ovs_nets), _get_kernel_bonds_slaves()) > cant we move most if this to validator and keep just 'ovs_switch.validator. ovs_nets can be regenerated only here and the logic on how to interpret the info is done here. the validator is currently nicely independent and does not depend on too much external stuff. It expects the info to flow in already. I think we should do an overall cleanup in this module (netswitch) and move things under the relevant switch, keeping here only switch common stuff. https://gerrit.ovirt.org/#/c/64212/4/lib/vdsm/network/ovs/validator.py File lib/vdsm/network/ovs/validator.py: PS4, Line 43: new_bond > this matches not only new bonds, but also ones we want to edit. Right, I can call it setup_bond or request_bond. It is related to the validation of usages in the sense that if a bond was mentioned for removal, but also mentioned in a network SB, we would not have detected this error. The addition is this: and 'remove' not in bonds[bond] PS4, Line 80: get('nic') > or get('nic', []), but i guess your code is more explicit Nope, that will not work when key exists with value of None. (Failed in the tests) -- To view, visit https://gerrit.ovirt.org/64212 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie1ed8805f7e1b84e784feef14c5378b41af1cbf1 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Edward Haas <edwa...@redhat.com> Gerrit-Reviewer: Edward Haas <edwa...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček <phora...@redhat.com> Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
