Mark Wu has posted comments on this change.

Change subject: Don't reconfigure vlan devs unless necessary.
......................................................................


Patch Set 3: I would prefer that you didn't submit this

(2 inline comments)

....................................................
File lib/vdsm/netinfo.py
Line 183:     for path in iglob(BONDING_OPTS % bonding):
Line 184:         if os.path.isfile(path):
Line 185:             with open(path) as optFile:
Line 186:                 opts.append((os.path.basename(path),
Line 187:                              optFile.read().rstrip().split(' ')[0]))
The value of some bonding options has two representations: text string and 
number.
You only collect the text string here. If the given option is using number, it 
can't match even though they're the same.

'mode=balance-rr'   should be equal to 'mode=0'
Line 188:     return frozenset(opts)
Line 189: 
Line 190: 
Line 191: def ports(bridge):


....................................................
File vdsm/netmodels.py
Line 188: 
Line 189:     def _options_running(self):
Line 190:         active_opts = netinfo.bondOpts(self.name)
Line 191:         return all(tuple(opt.split('=', 1)) in active_opts for
Line 192:                    opt in self.options)
isn't self.options a string,  so you need  self.options.split(' ')
Line 193: 
Line 194:     def remove(self):
Line 195:         logging.debug('Removing bond %r', self)
Line 196:         self.configurator.removeBond(self)


-- 
To view, visit http://gerrit.ovirt.org/17491
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id3075a2e65f06416c840c6a98689b77555b22e5d
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon <asegu...@redhat.com>
Gerrit-Reviewer: Mark Wu <wu...@linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to