Hi Andreas, On Fri, Jun 19, 2009 at 08:19:29AM -0400, Cole Robinson wrote: > Actually, we had a similar bug reported against fedora, which I just fixed > upstream yesterday!: > > http://hg.et.redhat.com/cgi-bin/hg-virt.cgi/applications/virtinst--devel/rev/503cd42936a9 > > That 'default_bridge' function is bogus, but it's part of the public API in > 'util' so we need to maintain it's behavior. Could you check if the attached patch fixes your problem, it's the upstream patch backported to 0.400.3? -- Guido
>From 52329c24b49389c0f72992a81bdad753e5e17647 Mon Sep 17 00:00:00 2001 From: =?utf-8?q?Guido=20G=C3=BCnther?= <a...@sigxcpu.org> Date: Sun, 21 Jun 2009 19:06:15 +0200 Subject: [PATCH] Closes: #533539
--- virtinst/VirtualNetworkInterface.py | 2 +- virtinst/_util.py | 50 +++++++++++++++++++++++++++++- virtinst/util.py | 58 +++++++--------------------------- 3 files changed, 62 insertions(+), 48 deletions(-) diff --git a/virtinst/VirtualNetworkInterface.py b/virtinst/VirtualNetworkInterface.py index 0e77519..6f3c3c4 100644 --- a/virtinst/VirtualNetworkInterface.py +++ b/virtinst/VirtualNetworkInterface.py @@ -127,7 +127,7 @@ class VirtualNetworkInterface(VirtualDevice.VirtualDevice): raise RuntimeError(msg) if not self.bridge and self.type == "bridge": - self.bridge = _util.default_bridge() + self.bridge = _util.default_bridge2(self.conn) def get_xml_config(self): src_xml = "" diff --git a/virtinst/_util.py b/virtinst/_util.py index ff5bc91..5169971 100644 --- a/virtinst/_util.py +++ b/virtinst/_util.py @@ -28,10 +28,12 @@ import os import re import commands import logging +import platform +import subprocess import libvirt -from virtinst import util +import virtinst.util as util from virtinst import _virtinst as _ def is_vdisk(path): @@ -176,6 +178,52 @@ def fetch_all_guests(conn): return (active, inactive) +def default_nic(): + """ + Return the default NIC to use, if one is specified. + """ + + dev = '' + + if platform.system() != 'SunOS': + return dev + + # XXX: fails without PRIV_XVM_CONTROL + proc = subprocess.Popen(['/usr/lib/xen/bin/xenstore-read', + 'device-misc/vif/default-nic'], stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + out = proc.stdout.readlines() + if len(out) > 0: + dev = out[0].rstrip() + + return dev + +def default_bridge2(conn = None): + if platform.system() == 'SunOS': + return ["bridge", default_nic()] + + dev = util.default_route() + + if (dev is not None and + (not conn or not is_uri_remote(conn.getURI()))): + # New style peth0 == phys dev, eth0 == bridge, eth0 == default route + if os.path.exists("/sys/class/net/%s/bridge" % dev): + return ["bridge", dev] + + # Old style, peth0 == phys dev, eth0 == netloop, xenbr0 == bridge, + # vif0.0 == netloop enslaved, eth0 == default route + try: + defn = int(dev[-1]) + except: + defn = -1 + + if (defn >= 0 and + os.path.exists("/sys/class/net/peth%d/brport" % defn) and + os.path.exists("/sys/class/net/xenbr%d/bridge" % defn)): + return ["bridge", "xenbr%d" % defn] + + return None + # # These functions accidentally ended up in the API under virtinst.util # diff --git a/virtinst/util.py b/virtinst/util.py index 2698b59..6308e5b 100644 --- a/virtinst/util.py +++ b/virtinst/util.py @@ -38,6 +38,7 @@ from sys import stderr import libvirt from virtinst import _virtinst as _ +import virtinst import CapabilitiesParser from User import User @@ -76,59 +77,24 @@ def default_route(nic = None): continue return None -def _default_nic(): - """Return the default NIC to use, if one is specified. - This is NOT part of the API and may change at will.""" - - dev = '' - - if platform.system() != 'SunOS': - return dev - - # XXX: fails without PRIV_XVM_CONTROL - proc = subprocess.Popen(['/usr/lib/xen/bin/xenstore-read', - 'device-misc/vif/default-nic'], stdout=subprocess.PIPE, - stderr=subprocess.PIPE) - out = proc.stdout.readlines() - if len(out) > 0: - dev = out[0].rstrip() - - return dev def default_bridge(): - if platform.system() == 'SunOS': - return _default_nic() - - rt = default_route() - if rt is None: - defn = None + ret = virtinst._util.default_bridge2(None) + if not ret: + # Maintain this behavior for back compat + ret = "xenbr0" else: - defn = int(rt[-1]) + ret = ret[1] - if defn is None: - return "xenbr0" - else: - return "xenbr%d"%(defn) + return ret def default_network(conn): - if platform.system() == 'SunOS': - return ["bridge", _default_nic()] - - dev = default_route() - - if dev is not None and not is_uri_remote(conn.getURI()): - # New style peth0 == phys dev, eth0 == bridge, eth0 == default route - if os.path.exists("/sys/class/net/%s/bridge" % dev): - return ["bridge", dev] - - # Old style, peth0 == phys dev, eth0 == netloop, xenbr0 == bridge, - # vif0.0 == netloop enslaved, eth0 == default route - defn = int(dev[-1]) - if os.path.exists("/sys/class/net/peth%d/brport" % defn) and \ - os.path.exists("/sys/class/net/xenbr%d/bridge" % defn): - return ["bridge", "xenbr%d" % defn] + ret = virtinst._util.default_bridge2(conn) + if not ret: + # FIXME: Check that this exists + ret = ["network", "default"] - return ["network", "default"] + return ret def default_connection(): if os.path.exists('/var/lib/xend'): -- 1.6.3.1