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