Dan Kenigsberg has posted comments on this change.

Change subject: BZ#811807 Enable network filtering
......................................................................


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

(1 inline comment)

....................................................
File vdsm/libvirtvm.py
Line 963:         if hasattr(self, 'filters'):
Line 964:             for filterName in self.filters:
Line 965:                 m = doc.createElement('filterref')
Line 966:                 m.setAttribute('filter', filterName)
Line 967:                 iface.appendChild(m)
I'm pretty sure we've seen that libvirt (currently?) supports only one 
filterref per nic, so at the moment the api is misleading.

Either change the API to accept only one, or open an RFE for libvirt to support 
more. Either way, this issue should be documented in the commit message.
PS I am not sure what would the semantics of multiple filterrefs be - OR? AND? 
Priority? I'm not sure it is going to fly so easily.
Line 968:         if hasattr(self, 'bootOrder'):
Line 969:             bootOrder = doc.createElement('boot')
Line 970:             bootOrder.setAttribute('order', self.bootOrder)
Line 971:             iface.appendChild(bootOrder)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I33b3dc7021997f4d0c1194049b1c12ac3528f067
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Moti Asayag <masa...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvov...@redhat.com>
Gerrit-Reviewer: Livnat Peer <lp...@redhat.com>
Gerrit-Reviewer: Moti Asayag <masa...@redhat.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