On 10/03/2014 04:27 PM, bbaude@redhat.com wrote:

From: Brent Baude <bbaude@redhat.com>

This patch removes the static QEMU URIs that are defined and replaces them with a function to lookup the system URI. This helps Kimchi work with a remote libvirtd.

--- src/kimchi/config.py.in | 4 ++-- src/kimchi/featuretests.py | 10 +++++----- src/kimchi/kvmusertests.py | 3 ++- src/kimchi/model/libvirtconnection.py | 25 +++++++++++++++++++++++++ src/kimchi/model/model.py | 6 +++--- src/kimchi/model/networks.py | 2 +- tests/test_model.py | 17 +++++++++-------- 7 files changed, 47 insertions(+), 20 deletions(-)

diff --git a/src/kimchi/config.py.in b/src/kimchi/config.py.in index 91e5f48..51cd442 100644 --- a/src/kimchi/config.py.in +++ b/src/kimchi/config.py.in @@ -23,7 +23,6 @@ import os import platform import threading

- from ConfigParser import SafeConfigParser

@@ -61,8 +60,9 @@ def get_version():

def find_qemu_binary(find_emulator=False): + from kimchi.model.libvirtconnection import LibvirtConnection try: - connect = libvirt.open('qemu:///system')

+ connect = libvirt.open(LibvirtConnection.getSystemURI())

You can just call: connect = libvirt.open(None) as the URI is not needed here.

except Exception, e: raise Exception("Unable to get qemu binary location: %s" % e) try: diff --git a/src/kimchi/featuretests.py b/src/kimchi/featuretests.py index 8964098..a2a942b 100644 --- a/src/kimchi/featuretests.py +++ b/src/kimchi/featuretests.py @@ -27,7 +27,7 @@ import threading

from lxml.builder import E

- +from kimchi.model.libvirtconnection import LibvirtConnection from kimchi.rollbackcontext import RollbackContext from kimchi.utils import kimchi_log

@@ -104,7 +104,7 @@ class FeatureTests(object): conn = None try: FeatureTests.disable_screen_error_logging() - conn = libvirt.open('qemu:///system')

+ conn = libvirt.open(LibvirtConnection.getSystemURI())

Here too.

dom = conn.defineXML(xml) dom.undefine() return True @@ -122,7 +122,7 @@ class FeatureTests(object): xml = ET.tostring(obj) return xml try: - conn = libvirt.open('qemu:///system')

+ conn = libvirt.open(LibvirtConnection.getSystemURI())

And here:

FeatureTests.disable_screen_error_logging() conn.findStoragePoolSources('netfs', _get_xml(), 0) except libvirt.libvirtError as e: @@ -174,7 +174,7 @@ class FeatureTests(object): def libvirt_support_fc_host(): try: FeatureTests.disable_screen_error_logging() - conn = libvirt.open('qemu:///system')

+ conn = libvirt.open(LibvirtConnection.getSystemURI())

conn = libvirt.open(None)

pool = None pool = conn.storagePoolDefineXML(SCSI_FC_XML, 0) except libvirt.libvirtError as e: @@ -194,7 +194,7 @@ class FeatureTests(object): with RollbackContext() as rollback: FeatureTests.disable_screen_error_logging() rollback.prependDefer(FeatureTests.enable_screen_error_logging) - conn = libvirt.open('qemu:///system')

+ conn = libvirt.open(LibvirtConnection.getSystemURI())

conn = libvirt.open(None)

rollback.prependDefer(conn.close) dom = conn.defineXML(SIMPLE_VM_XML) rollback.prependDefer(dom.undefine) diff --git a/src/kimchi/kvmusertests.py b/src/kimchi/kvmusertests.py index 1ac2beb..ae1df25 100644 --- a/src/kimchi/kvmusertests.py +++ b/src/kimchi/kvmusertests.py @@ -24,6 +24,7 @@ import libvirt

from kimchi.rollbackcontext import RollbackContext +from kimchi.model.libvirtconnection import LibvirtConnection

class UserTests(object): @@ -49,7 +50,7 @@ class UserTests(object):

xml = cls.SIMPLE_VM_XML % (vm_name, vm_uuid) with RollbackContext() as rollback: - conn = libvirt.open('qemu:///system')

+ conn = libvirt.open(LibvirtConnection.getSystemURI())

conn = libvirt.open(None)

rollback.prependDefer(conn.close) dom = conn.defineXML(xml) rollback.prependDefer(dom.undefine) diff --git a/src/kimchi/model/libvirtconnection.py b/src/kimchi/model/libvirtconnection.py index 80348a3..bac276f 100644 --- a/src/kimchi/model/libvirtconnection.py +++ b/src/kimchi/model/libvirtconnection.py @@ -117,3 +117,28 @@ class LibvirtConnection(object): # However the values need to be considered wisely to not affect # hosts which are hosting a lot of virtual machines return conn +

+ @staticmethod

I don't think it needs to be static anymore as it will be only used by LibvirtConnection instance.

+ def getSystemURI(): + """ + This method looks up the livbirt URI for the system instead of + relying on a statically defined URI. + """ + try: + uriconn = libvirt.openReadOnly(None) + return uriconn.getURI() + except: + err = 'Unable to connect to libvirtd to determine the URI' + kimchi_log.error(err) +

You need to close the libvirt connection

+ @staticmethod

Same here about staticmethod.

+ def isQemuURI(libvirt_uri): + """ + This method will return True or Value when the system libvirt + URI is a qemu based URI. For example: + qemu:///system or qemu+tcp://someipaddress/system + """

+ if libvirt_uri.startswith('qemu'):

if self.libvirt_uri.startswith('qemu'):

+ return True + else: + return False

diff --git a/src/kimchi/model/model.py b/src/kimchi/model/model.py index ac70852..b8b2f9d 100644 --- a/src/kimchi/model/model.py +++ b/src/kimchi/model/model.py @@ -38,12 +38,12 @@ DEFAULT_POOLS = {'default': {'path': '/var/lib/libvirt/images'},

class Model(BaseModel): - def __init__(self, libvirt_uri='qemu:///system', objstore_loc=None): + def __init__(self, libvirt_uri=LibvirtConnection.getSystemURI(),

Defaults libvirt_uri to None + def __init__(self, libvirt_uri=None, And only get the system URI when it is None: if libvirt_uri is None: libvirt_uri = self.getSystemURI()

+ objstore_loc=None): self.objstore = ObjectStore(objstore_loc) self.conn = LibvirtConnection(libvirt_uri) kargs = {'objstore': self.objstore, 'conn': self.conn} - - if 'qemu:///' in libvirt_uri:

+ if (self.conn.isQemuURI(libvirt_uri)):

if self.isQemURI():

for pool_name, pool_arg in DEFAULT_POOLS.iteritems(): self._default_pool_check(pool_name, pool_arg)

diff --git a/src/kimchi/model/networks.py b/src/kimchi/model/networks.py index 0ea9449..f01bed0 100644 --- a/src/kimchi/model/networks.py +++ b/src/kimchi/model/networks.py @@ -41,7 +41,7 @@ KIMCHI_BRIDGE_PREFIX = 'kb' class NetworksModel(object): def __init__(self, **kargs): self.conn = kargs['conn'] - if 'qemu:///' in self.conn.get().getURI():

+ if self.conn.isQemuURI(self.conn.get().getURI()):

if self.conn.isQemuURI();

self._default_network_check()

def _default_network_check(self): diff --git a/tests/test_model.py b/tests/test_model.py index 1f2e79c..e23acf3 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -44,6 +44,7 @@ from kimchi.model import model from kimchi.rollbackcontext import RollbackContext from kimchi.utils import add_task

+from kimchi.model.libvirtconnection import LibvirtConnection

invalid_repository_urls = ['www.fedora.org', # missing protocol '://www.fedora.org', # missing protocol @@ -457,7 +458,7 @@ class ModelTests(unittest.TestCase):

@unittest.skipUnless(utils.running_as_root(), 'Must be run as root') def test_storagepool(self): - inst = model.Model('qemu:///system', self.tmp_store)

+ inst = model.Model(LibvirtConnection.getSystemURI(), self.tmp_store)

Only defaults libvirt_uri to None and let LibvirtConnection handle it properly + inst = model.Model(None, self.tmp_store) Do it for all the occurrences of qemu:///system

poolDefs = [ {'type': 'dir', @@ -524,7 +525,7 @@ class ModelTests(unittest.TestCase):

@unittest.skipUnless(utils.running_as_root(), 'Must be run as root') def test_storagevolume(self): - inst = model.Model('qemu:///system', self.tmp_store) + inst = model.Model(LibvirtConnection.getSystemURI(), self.tmp_store) with RollbackContext() as rollback: path = '/tmp/kimchi-images' @@ -742,7 +743,7 @@ class ModelTests(unittest.TestCase):

@unittest.skipUnless(utils.running_as_root(), 'Must be run as root') def test_template_clone(self): - inst = model.Model('qemu:///system', + inst = model.Model(LibvirtConnection.getSystemURI(), objstore_loc=self.tmp_store) with RollbackContext() as rollback: orig_params = {'name': 'test-template', 'memory': 1024, @@ -761,7 +762,7 @@ class ModelTests(unittest.TestCase):

@unittest.skipUnless(utils.running_as_root(), 'Must be run as root') def test_template_update(self): - inst = model.Model('qemu:///system', + inst = model.Model(LibvirtConnection.getSystemURI(), objstore_loc=self.tmp_store) with RollbackContext() as rollback: net_name = 'test-network' @@ -801,7 +802,7 @@ class ModelTests(unittest.TestCase): 'new-test', params)

def test_vm_edit(self): - inst = model.Model('qemu:///system', + inst = model.Model(LibvirtConnection.getSystemURI(), objstore_loc=self.tmp_store)

orig_params = {'name': 'test', 'memory': '1024', 'cpus': '1', @@ -914,7 +915,7 @@ class ModelTests(unittest.TestCase):

@unittest.skipUnless(utils.running_as_root(), 'Must be run as root') def test_network(self): - inst = model.Model('qemu:///system', self.tmp_store) + inst = model.Model(LibvirtConnection.getSystemURI(), self.tmp_store)

with RollbackContext() as rollback:

@@ -1240,7 +1241,7 @@ class ModelTests(unittest.TestCase):

@unittest.skipUnless(utils.running_as_root(), 'Must be run as root') def test_get_hostinfo(self): - inst = model.Model('qemu:///system', + inst = model.Model(LibvirtConnection.getSystemURI(), objstore_loc=self.tmp_store) info = inst.host_lookup() distro, version, codename = platform.linux_distribution() @@ -1279,7 +1280,7 @@ class ModelTests(unittest.TestCase):

@unittest.skipUnless(utils.running_as_root(), 'Must be run as root') def test_deep_scan(self): - inst = model.Model('qemu:///system', + inst = model.Model(LibvirtConnection.getSystemURI(), objstore_loc=self.tmp_store) with RollbackContext() as rollback: path = '/tmp/kimchi-images/tmpdir'