Change in vdsm[master]: add/del network - add bridgesless network
danken at redhat.com
danken at redhat.com
Wed Feb 15 15:45:55 UTC 2012
Dan Kenigsberg has posted comments on this change.
Change subject: add/del network - add bridgesless network
......................................................................
Patch Set 11: I would prefer that you didn't submit this
(6 inline comments)
minor comments, mostly
....................................................
File vdsm/clientIF.py
Line 119: # remove bridged networks that their bridge not exists
ouch, this seems unrelated to bridgeless, and should probably go into vdsm-restore-net script. Either way, please separate into another patch.
....................................................
File vdsm/configNetwork.py
Line 29: from xml.sax.saxutils import escape
import order - libvirt is not (yet!) part of python's stdlib
Line 586: bondingOptions=bondingOptions, bridged=bridged, skipLibvirt=skipLibvirt)
please break long lines
....................................................
File vdsm/netinfo.py
Line 26: from xml.dom import minidom
nonstandard order
Line 245: for netname in getnetworks().keys():
I'd rather cache the results of getnetworks, poor libvirt should not suffer so much.
Line 387: for netname in getnetworks().keys():
use iteritems.
but list comprehension is even cooler!
return [ netname for (netname, net) in getnetworks().iteritems() if 'bridge' in net ]
--
To view, visit http://gerrit.ovirt.org/848
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id7a3efea92312ac628e0373a5c29fbb1669058f4
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shavivi at redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky at redhat.com>
Gerrit-Reviewer: Peter V. Saveliev <peet at redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Shahar Havivi <shavivi at redhat.com>
More information about the vdsm-patches
mailing list