Dan Kenigsberg has posted comments on this change.
Change subject: improve add/del bridgesless network ......................................................................
Patch Set 3: I would prefer that you didn't submit this
(6 inline comments)
.................................................... Commit Message Line 11: it seems that this patch fixes several problems. If so please split it, and explain the solved issues.
.................................................... File vdsm/configNetwork.py Line 746: removeLibvirtNetwork(vlandev) this is very confusing, since vlandev is NOT a libvirt network. why is this needed?
.................................................... File vdsm/netinfo.py Line 93: if os.path.exists(os.path.join(bridgePath)): why do you expect this to fail? I think that this function should be called only on existing bridges. We can think, however, to change the semantics, catch the exception and return []
Line 274: # interface already removed! if the network is removed, why should we report it at all?
.................................................... File vdsm_reg/deployUtil.py.in Line 42: sys.path.append("/usr/share/vdsm") messing with sys.path during runtime is evil. let us try hard to avoid this.
oh, and import would fail if /usr/share/vdsm does not exist yet.
Line 900: configNetwork.createLibvirtNetwork(mgtBridge, bridged=True) I would very much prefer it if skipLibvirt is adhered to, instead.
now you create a network only to delete it one line below...
-- To view, visit http://gerrit.ovirt.org/3615 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7f170eaced2d6f756b84cb52e9efce6dc9722405 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Douglas Schilling Landgraf dougsland@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsland@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Shahar Havivi shavivi@redhat.com