[Kimchi-devel] [PATCH] Remove static qemu URIs and add function to lookup the system URI
Aline Manera
alinefm at linux.vnet.ibm.com
Mon Oct 6 15:02:41 UTC 2014
On 10/03/2014 04:27 PM, bbaude at redhat.com wrote:
> From: Brent Baude <bbaude at 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'
More information about the Kimchi-devel
mailing list