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