Yeela Kaplan has uploaded a new change for review.
Change subject: caps: fix unpacking of list ......................................................................
caps: fix unpacking of list
getVdsCapabilities fails with 'too many values to unpack'
Change-Id: I20d4b8b6e3147a29aebcbc7a68b9c92d23aa4307 Signed-off-by: Yeela Kaplan ykaplan@redhat.com --- M vdsm/caps.py 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/21/46321/1
diff --git a/vdsm/caps.py b/vdsm/caps.py index 94b7c34..17feecd 100644 --- a/vdsm/caps.py +++ b/vdsm/caps.py @@ -98,7 +98,7 @@ not port.startswith('vnet')] if not host_ports: # Port-less bridge continue - iface, = host_ports + iface = host_ports[0] if iface in caps['vlans']: vlan_id = caps['vlans'][iface]['vlanid'] iface = caps['vlans'][iface]['iface']
automation@ovirt.org has posted comments on this change.
Change subject: caps: fix unpacking of list ......................................................................
Patch Set 1:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Ido Barkan has posted comments on this change.
Change subject: caps: fix unpacking of list ......................................................................
Patch Set 1: Code-Review-1
(1 comment)
https://gerrit.ovirt.org/#/c/46321/1/vdsm/caps.py File vdsm/caps.py:
Line 97: host_ports = [port for port in attrs['ports'] if Line 98: not port.startswith('vnet')] Line 99: if not host_ports: # Port-less bridge Line 100: continue Line 101: iface = host_ports[0] How did this happen? Can you reproduce reliably? There shouldn't be in oVirt a bridge with more then one 'non-vnet' ports. This is why this strict unpacking was there. I agree that the failure us a bit misleading, but I think we should at least shout in the logs when this is happening. Line 102: if iface in caps['vlans']: Line 103: vlan_id = caps['vlans'][iface]['vlanid'] Line 104: iface = caps['vlans'][iface]['iface'] Line 105: iface_qdiscs = qdiscs.get(iface)
automation@ovirt.org has posted comments on this change.
Change subject: caps: raise if bridge has more than one port ......................................................................
Patch Set 2:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Ido Barkan has posted comments on this change.
Change subject: caps: raise if bridge has more than one port ......................................................................
Patch Set 2: Code-Review-1
(2 comments)
https://gerrit.ovirt.org/#/c/46321/2/vdsm/caps.py File vdsm/caps.py:
Line 97: host_ports = [port for port in attrs['ports'] if Line 98: not port.startswith('vnet')] Line 99: if not host_ports: # Port-less bridge Line 100: continue Line 101: if len(host_ports) > 1: to keep the original meaning you should test for '== 1' Line 102: raise RuntimeError('bridge with more than one port') Line 103: iface, = host_ports Line 104: if iface in caps['vlans']: Line 105: vlan_id = caps['vlans'][iface]['vlanid']
Line 99: if not host_ports: # Port-less bridge Line 100: continue Line 101: if len(host_ports) > 1: Line 102: raise RuntimeError('bridge with more than one port') Line 103: iface, = host_ports now, this defense is not needed anymore so you can just extract hots_ports[0], otherwise the exception would be already raised. Line 104: if iface in caps['vlans']: Line 105: vlan_id = caps['vlans'][iface]['vlanid'] Line 106: iface = caps['vlans'][iface]['iface'] Line 107: iface_qdiscs = qdiscs.get(iface)
Yeela Kaplan has posted comments on this change.
Change subject: caps: raise if bridge has more than one port ......................................................................
Patch Set 2:
(2 comments)
https://gerrit.ovirt.org/#/c/46321/2/vdsm/caps.py File vdsm/caps.py:
Line 97: host_ports = [port for port in attrs['ports'] if Line 98: not port.startswith('vnet')] Line 99: if not host_ports: # Port-less bridge Line 100: continue Line 101: if len(host_ports) > 1:
to keep the original meaning you should test for '== 1'
Done Line 102: raise RuntimeError('bridge with more than one port') Line 103: iface, = host_ports Line 104: if iface in caps['vlans']: Line 105: vlan_id = caps['vlans'][iface]['vlanid']
Line 99: if not host_ports: # Port-less bridge Line 100: continue Line 101: if len(host_ports) > 1: Line 102: raise RuntimeError('bridge with more than one port') Line 103: iface, = host_ports
now, this defense is not needed anymore so you can just extract hots_ports[
Done Line 104: if iface in caps['vlans']: Line 105: vlan_id = caps['vlans'][iface]['vlanid'] Line 106: iface = caps['vlans'][iface]['iface'] Line 107: iface_qdiscs = qdiscs.get(iface)
automation@ovirt.org has posted comments on this change.
Change subject: caps: raise if bridge has more than one port ......................................................................
Patch Set 3:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Ido Barkan has posted comments on this change.
Change subject: caps: raise if bridge has more than one port ......................................................................
Patch Set 3: Code-Review+1
Yeela Kaplan has posted comments on this change.
Change subject: caps: raise if bridge has more than one port ......................................................................
Patch Set 3: Verified+1
Dan Kenigsberg has posted comments on this change.
Change subject: caps: raise if bridge has more than one port ......................................................................
Patch Set 3: Code-Review-1
(1 comment)
https://gerrit.ovirt.org/#/c/46321/3/vdsm/caps.py File vdsm/caps.py:
Line 100: continue Line 101: if len(host_ports) == 1: Line 102: iface = host_ports[0] Line 103: else: Line 104: raise RuntimeError('bridge with more than one port') a more useful log would list the name of the offending bridge and its ports. Line 105: Line 106: if iface in caps['vlans']: Line 107: vlan_id = caps['vlans'][iface]['vlanid'] Line 108: iface = caps['vlans'][iface]['iface']
automation@ovirt.org has posted comments on this change.
Change subject: caps: raise if bridge has more than one port ......................................................................
Patch Set 4:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Ido Barkan has posted comments on this change.
Change subject: caps: raise if bridge has more than one port ......................................................................
Patch Set 4: Code-Review+1
vdsm-patches@lists.fedorahosted.org
-
automation＠ovirt.org -
Dan Kenigsberg -
ibarkan＠redhat.com -
ykaplan＠redhat.com