[PATCH 1/2] Dynamically generate template parameters for different distros
We define hard coded template for different distros. It's not flexible to generate templates for other platforms or new distros. This patch changes to dynamically generate vm template. With this change, it can support new distro, like Fedora20, without any modifications. Signed-off-by: Mark Wu <wudxw@linux.vnet.ibm.com> --- src/kimchi/osinfo.py | 174 ++++++++++------------------------------------- src/kimchi/vmtemplate.py | 2 +- tests/test_osinfo.py | 28 ++++++-- 3 files changed, 60 insertions(+), 144 deletions(-) diff --git a/src/kimchi/osinfo.py b/src/kimchi/osinfo.py index 20a93a6..b141d9e 100644 --- a/src/kimchi/osinfo.py +++ b/src/kimchi/osinfo.py @@ -22,136 +22,23 @@ import copy import os +from distutils.version import LooseVersion + + +common_spec = {'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, 'memory': 1024, + 'disks': [{'index': 0, 'size': 10}], 'cdrom_bus': 'ide', + 'cdrom_index': 2} + + +modern_spec = dict(common_spec, disk_bus='virtio', nic_model='virtio') + + +old_spec = dict(common_spec, disk_bus='ide', nic_model='e1000') + + +modern_version_bases = {'debian': '6.0', 'ubuntu': '7.10', 'opensuse': '10.3', + 'centos': '5.3', 'rhel': '6.0', 'fedora': '16'} -osinfo = [ - # Entries are searched in order and the first match will be returned - ('debian', { - 'version': lambda d,v: bool(d == 'debian' and v in ('6.0', '7.0')), - 'icon': 'images/icon-debian.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'virtio', 'nic_model': 'virtio', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('debian-old', { - 'version': lambda d,v: bool(d == 'debian'), - 'icon': 'images/icon-debian.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'ide', 'nic_model': 'e1000', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('ubuntu', { - 'version': lambda d,v: bool(d == 'ubuntu' and v in - ('7.10', '8.04', '8.10', '9.04', '9.10', '10.04', '10.10', - '11.04', '11.10', '12.04', '12.10', '13.04', '13.10')), - 'icon': 'images/icon-ubuntu.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'virtio', 'nic_model': 'virtio', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('ubuntu-old', { - 'version': lambda d,v: bool(d == 'ubuntu'), - 'icon': 'images/icon-ubuntu.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'ide', 'nic_model': 'e1000', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('opensuse', { - 'version': lambda d,v: bool(d == 'opensuse' and v in - ('10.3', '11.0', '11.1', '11.2', '11.3', '11.4', '12.1', '12.2', - '12.3',)), - 'icon': 'images/icon-opensuse.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'virtio', 'nic_model': 'virtio', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('opensuse-old', { - 'version': lambda d,v: bool(d == 'opensuse'), - 'icon': 'images/icon-opensuse.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'ide', 'nic_model': 'e1000', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('fedora', { - 'version': lambda d,v: bool(d == 'fedora' and v in - ('16', '17', '18', '19')), - 'icon': 'images/icon-fedora.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'virtio', 'nic_model': 'virtio', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('fedora-old', { - 'version': lambda d,v: bool(d == 'fedora'), - 'icon': 'images/icon-fedora.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'ide', 'nic_model': 'e1000', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('rhel', { - 'version': lambda d,v: bool(d == 'rhel' and - v.startswith('6.')), - 'icon': 'images/icon-vm.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'virtio', 'nic_model': 'virtio', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('rhel-old', { - 'version': lambda d,v: bool(d == 'rhel'), - 'icon': 'images/icon-vm.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'ide', 'nic_model': 'e1000', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('centos', { - 'version': lambda d,v: bool(d == 'centos' and - (v in ('5.3', '5.4', '5.5') or v.startswith('6.'))), - 'icon': 'images/icon-centos.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'virtio', 'nic_model': 'virtio', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('centos-old', { - 'version': lambda d,v: bool(d == 'centos'), - 'icon': 'images/icon-centos.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'ide', 'nic_model': 'e1000', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('unknown', { - 'version': lambda d,v: True, - 'icon': 'images/icon-vm.png', - 'os_distro': 'unknown', 'os_version': 'unknown', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'cdrom': '', - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'ide', 'nic_model': 'e1000', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), -] isolinks = { 'debian': { @@ -181,13 +68,22 @@ def lookup(distro, version): 'defaults' and merging the parameters given for the identified OS. If known, a link to a remote install CD is added. """ - for name, entry in osinfo: - # Test if this entry is a valid match - if entry['version'](distro, version): - params = copy.copy(defaults) - params['os_distro'] = distro - params['os_version'] = version - params.update(entry) - params['cdrom'] = isolinks.get(distro, {}).get(version, '') - del params['version'] # Don't pass around the version function - return (name, params) + params = copy.copy(defaults) + params['os_distro'] = distro + params['os_version'] = version + params['cdrom'] = isolinks.get(distro, {}).get(version, '') + + for name, base_version in modern_version_bases.iteritems(): + if name == distro: + params['icon'] = 'images/icon-%s.png' % distro + if LooseVersion(version) >= LooseVersion(base_version): + params.update(modern_spec) + else: + params.update(old_spec) + break + else: + params['icon'] = 'images/icon-vm.png' + params['os_distro'] = params['os_version'] = "unknown" + params.update(old_spec) + + return params diff --git a/src/kimchi/vmtemplate.py b/src/kimchi/vmtemplate.py index dd43faa..f92af49 100644 --- a/src/kimchi/vmtemplate.py +++ b/src/kimchi/vmtemplate.py @@ -70,7 +70,7 @@ class VMTemplate(object): # Fetch defaults based on the os distro and version os_distro = args.get('os_distro', iso_distro) os_version = args.get('os_version', iso_version) - name, entry = osinfo.lookup(os_distro, os_version) + entry = osinfo.lookup(os_distro, os_version) self.info.update(entry) # Override with the passed in parameters diff --git a/tests/test_osinfo.py b/tests/test_osinfo.py index f92567d..da3e1d2 100644 --- a/tests/test_osinfo.py +++ b/tests/test_osinfo.py @@ -25,16 +25,36 @@ from kimchi.osinfo import * class OSInfoTests(unittest.TestCase): def test_default_lookup(self): - name, entry = lookup(None, None) - self.assertEquals(name, 'unknown') + entry = lookup(None, None) self.assertEquals('unknown', entry['os_distro']) self.assertEquals('unknown', entry['os_version']) self.assertEquals('default', entry['network']) def test_fedora_lookup(self): cd = 'http://fedora.mirrors.tds.net/pub/fedora/releases/17/Live/x86_64/Fedora-17-x...' - name, entry = lookup('fedora', '17') - self.assertEquals(name, 'fedora') + entry = lookup('fedora', '17') self.assertEquals(10, entry['disks'][0]['size']) self.assertEquals(cd, entry['cdrom']) self.assertEquals('/storagepools/default', entry['storagepool']) + + def test_old_distros(self): + old_versions = {'debian': '5.0', 'ubuntu': '7.04', 'opensuse': '10.1', + 'centos': '5.1', 'rhel': '5.1', 'fedora': '15'} + for distro, version in old_versions.iteritems(): + entry = lookup(distro, version) + self.assertEquals(entry['disk_bus'], 'ide') + self.assertEquals(entry['nic_model'], 'e1000') + + def test_modern_bases(self): + for distro, version in modern_version_bases.iteritems(): + entry = lookup(distro, version) + self.assertEquals(entry['disk_bus'], 'virtio') + self.assertEquals(entry['nic_model'], 'virtio') + + def test_modern_distros(self): + old_versions = {'debian': '7.0', 'ubuntu': '12.04', 'opensuse': '12.3', + 'centos': '6.4', 'rhel': '6.3', 'fedora': '18'} + for distro, version in old_versions.iteritems(): + entry = lookup(distro, version) + self.assertEquals(entry['disk_bus'], 'virtio') + self.assertEquals(entry['nic_model'], 'virtio') -- 1.8.3.1
VM on power machine requires different parameters from X86 machine. So kimchi should provide different template according to the architecture. IDE bus is not available on power platform. So change the device bus of cdrom to scsi instead of ide for VM on power. The same problem exists for old nic model, and change it to rtl8139 instead. Signed-off-by: Mark Wu <wudxw@linux.vnet.ibm.com> --- src/kimchi/osinfo.py | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/src/kimchi/osinfo.py b/src/kimchi/osinfo.py index b141d9e..62c1bbf 100644 --- a/src/kimchi/osinfo.py +++ b/src/kimchi/osinfo.py @@ -25,6 +25,9 @@ import os from distutils.version import LooseVersion +SUPPORTED_ARCHS = {'x86': ('i386', 'x86_64'), 'power': ('ppc', 'ppc64')} + + common_spec = {'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, 'memory': 1024, 'disks': [{'index': 0, 'size': 10}], 'cdrom_bus': 'ide', 'cdrom_index': 2} @@ -33,7 +36,15 @@ common_spec = {'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, 'memory': 1024, modern_spec = dict(common_spec, disk_bus='virtio', nic_model='virtio') -old_spec = dict(common_spec, disk_bus='ide', nic_model='e1000') +template_specs = {'x86': {'old': dict(common_spec, disk_bus='ide', + nic_model='e1000'), + 'modern': dict(common_spec, disk_bus='virtio', + nic_model='virtio')}, + 'power': {'old': dict(common_spec, disk_bus='scsi', + nic_model='rtl8139', cdrom_bus='scsi'), + 'modern': dict(common_spec, disk_bus='virtio', + nic_model='virtio', + cdrom_bus='scsi')}} modern_version_bases = {'debian': '6.0', 'ubuntu': '7.10', 'opensuse': '10.3', @@ -61,6 +72,13 @@ defaults = {'network': 'default', 'storagepool': '/storagepools/default', 'domain': 'kvm', 'arch': os.uname()[4] } + +def _get_arch(): + for arch, sub_archs in SUPPORTED_ARCHS.iteritems(): + if os.uname()[4] in sub_archs: + return arch + + def lookup(distro, version): """ Lookup all parameters needed to run a VM of a known or unknown operating @@ -72,18 +90,19 @@ def lookup(distro, version): params['os_distro'] = distro params['os_version'] = version params['cdrom'] = isolinks.get(distro, {}).get(version, '') + arch = _get_arch() for name, base_version in modern_version_bases.iteritems(): if name == distro: params['icon'] = 'images/icon-%s.png' % distro if LooseVersion(version) >= LooseVersion(base_version): - params.update(modern_spec) + params.update(template_specs[arch]['modern']) else: - params.update(old_spec) + params.update(template_specs[arch]['old']) break else: params['icon'] = 'images/icon-vm.png' params['os_distro'] = params['os_version'] = "unknown" - params.update(old_spec) + params.update(template_specs[arch]['old']) return params -- 1.8.3.1
Reviewed-By: Ramon Medeiros <ramonn@br.ibm.com>
Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 12/23/2013 10:54 PM, Mark Wu wrote:
VM on power machine requires different parameters from X86 machine. So kimchi should provide different template according to the architecture. IDE bus is not available on power platform. So change the device bus of cdrom to scsi instead of ide for VM on power. The same problem exists for old nic model, and change it to rtl8139 instead.
Signed-off-by: Mark Wu <wudxw@linux.vnet.ibm.com> --- src/kimchi/osinfo.py | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-)
diff --git a/src/kimchi/osinfo.py b/src/kimchi/osinfo.py index b141d9e..62c1bbf 100644 --- a/src/kimchi/osinfo.py +++ b/src/kimchi/osinfo.py @@ -25,6 +25,9 @@ import os from distutils.version import LooseVersion
+SUPPORTED_ARCHS = {'x86': ('i386', 'x86_64'), 'power': ('ppc', 'ppc64')} + + common_spec = {'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, 'memory': 1024, 'disks': [{'index': 0, 'size': 10}], 'cdrom_bus': 'ide', 'cdrom_index': 2} @@ -33,7 +36,15 @@ common_spec = {'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, 'memory': 1024, modern_spec = dict(common_spec, disk_bus='virtio', nic_model='virtio')
-old_spec = dict(common_spec, disk_bus='ide', nic_model='e1000') +template_specs = {'x86': {'old': dict(common_spec, disk_bus='ide', + nic_model='e1000'), + 'modern': dict(common_spec, disk_bus='virtio', + nic_model='virtio')}, + 'power': {'old': dict(common_spec, disk_bus='scsi', + nic_model='rtl8139', cdrom_bus='scsi'), + 'modern': dict(common_spec, disk_bus='virtio', + nic_model='virtio', + cdrom_bus='scsi')}}
modern_version_bases = {'debian': '6.0', 'ubuntu': '7.10', 'opensuse': '10.3', @@ -61,6 +72,13 @@ defaults = {'network': 'default', 'storagepool': '/storagepools/default', 'domain': 'kvm', 'arch': os.uname()[4] }
+ +def _get_arch(): + for arch, sub_archs in SUPPORTED_ARCHS.iteritems(): + if os.uname()[4] in sub_archs: + return arch + + def lookup(distro, version): """ Lookup all parameters needed to run a VM of a known or unknown operating @@ -72,18 +90,19 @@ def lookup(distro, version): params['os_distro'] = distro params['os_version'] = version params['cdrom'] = isolinks.get(distro, {}).get(version, '') + arch = _get_arch()
for name, base_version in modern_version_bases.iteritems(): if name == distro: params['icon'] = 'images/icon-%s.png' % distro if LooseVersion(version) >= LooseVersion(base_version): - params.update(modern_spec) + params.update(template_specs[arch]['modern']) else: - params.update(old_spec) + params.update(template_specs[arch]['old']) break else: params['icon'] = 'images/icon-vm.png' params['os_distro'] = params['os_version'] = "unknown" - params.update(old_spec) + params.update(template_specs[arch]['old'])
return params
on 2013/12/24 08:54, Mark Wu wrote:
We define hard coded template for different distros. It's not flexible to generate templates for other platforms or new distros.
This patch changes to dynamically generate vm template. With this change, it can support new distro, like Fedora20, without any modifications.
Signed-off-by: Mark Wu <wudxw@linux.vnet.ibm.com> --- src/kimchi/osinfo.py | 174 ++++++++++------------------------------------- src/kimchi/vmtemplate.py | 2 +- tests/test_osinfo.py | 28 ++++++-- 3 files changed, 60 insertions(+), 144 deletions(-)
diff --git a/src/kimchi/osinfo.py b/src/kimchi/osinfo.py index 20a93a6..b141d9e 100644 --- a/src/kimchi/osinfo.py +++ b/src/kimchi/osinfo.py @@ -22,136 +22,23 @@
import copy import os +from distutils.version import LooseVersion + + +common_spec = {'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, 'memory': 1024, + 'disks': [{'index': 0, 'size': 10}], 'cdrom_bus': 'ide', + 'cdrom_index': 2} + + +modern_spec = dict(common_spec, disk_bus='virtio', nic_model='virtio') + + +old_spec = dict(common_spec, disk_bus='ide', nic_model='e1000') + + +modern_version_bases = {'debian': '6.0', 'ubuntu': '7.10', 'opensuse': '10.3', + 'centos': '5.3', 'rhel': '6.0', 'fedora': '16'}
-osinfo = [ - # Entries are searched in order and the first match will be returned - ('debian', { - 'version': lambda d,v: bool(d == 'debian' and v in ('6.0', '7.0')), - 'icon': 'images/icon-debian.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'virtio', 'nic_model': 'virtio', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('debian-old', { - 'version': lambda d,v: bool(d == 'debian'), - 'icon': 'images/icon-debian.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'ide', 'nic_model': 'e1000', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('ubuntu', { - 'version': lambda d,v: bool(d == 'ubuntu' and v in - ('7.10', '8.04', '8.10', '9.04', '9.10', '10.04', '10.10', - '11.04', '11.10', '12.04', '12.10', '13.04', '13.10')), - 'icon': 'images/icon-ubuntu.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'virtio', 'nic_model': 'virtio', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('ubuntu-old', { - 'version': lambda d,v: bool(d == 'ubuntu'), - 'icon': 'images/icon-ubuntu.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'ide', 'nic_model': 'e1000', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('opensuse', { - 'version': lambda d,v: bool(d == 'opensuse' and v in - ('10.3', '11.0', '11.1', '11.2', '11.3', '11.4', '12.1', '12.2', - '12.3',)), - 'icon': 'images/icon-opensuse.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'virtio', 'nic_model': 'virtio', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('opensuse-old', { - 'version': lambda d,v: bool(d == 'opensuse'), - 'icon': 'images/icon-opensuse.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'ide', 'nic_model': 'e1000', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('fedora', { - 'version': lambda d,v: bool(d == 'fedora' and v in - ('16', '17', '18', '19')), - 'icon': 'images/icon-fedora.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'virtio', 'nic_model': 'virtio', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('fedora-old', { - 'version': lambda d,v: bool(d == 'fedora'), - 'icon': 'images/icon-fedora.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'ide', 'nic_model': 'e1000', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('rhel', { - 'version': lambda d,v: bool(d == 'rhel' and - v.startswith('6.')), - 'icon': 'images/icon-vm.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'virtio', 'nic_model': 'virtio', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('rhel-old', { - 'version': lambda d,v: bool(d == 'rhel'), - 'icon': 'images/icon-vm.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'ide', 'nic_model': 'e1000', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('centos', { - 'version': lambda d,v: bool(d == 'centos' and - (v in ('5.3', '5.4', '5.5') or v.startswith('6.'))), - 'icon': 'images/icon-centos.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'virtio', 'nic_model': 'virtio', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('centos-old', { - 'version': lambda d,v: bool(d == 'centos'), - 'icon': 'images/icon-centos.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'ide', 'nic_model': 'e1000', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('unknown', { - 'version': lambda d,v: True, - 'icon': 'images/icon-vm.png', - 'os_distro': 'unknown', 'os_version': 'unknown', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'cdrom': '', - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'ide', 'nic_model': 'e1000', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), -]
isolinks = { 'debian': { @@ -181,13 +68,22 @@ def lookup(distro, version): 'defaults' and merging the parameters given for the identified OS. If known, a link to a remote install CD is added. """ - for name, entry in osinfo: - # Test if this entry is a valid match - if entry['version'](distro, version): - params = copy.copy(defaults) - params['os_distro'] = distro - params['os_version'] = version - params.update(entry) - params['cdrom'] = isolinks.get(distro, {}).get(version, '') - del params['version'] # Don't pass around the version function - return (name, params) + params = copy.copy(defaults) + params['os_distro'] = distro + params['os_version'] = version + params['cdrom'] = isolinks.get(distro, {}).get(version, '') + + for name, base_version in modern_version_bases.iteritems(): + if name == distro: + params['icon'] = 'images/icon-%s.png' % distro + if LooseVersion(version) >= LooseVersion(base_version): + params.update(modern_spec) + else: + params.update(old_spec) + break + else: + params['icon'] = 'images/icon-vm.png' + params['os_distro'] = params['os_version'] = "unknown" + params.update(old_spec) + Since modern_version_bases is a dict, and "if name == distro" is a exact match, it seems traversing the whole dict is not needed. Usually we just lookup value through the key.
if distro in modern_version_bases: base_version = modern_version_bases[distro] # LooseVersion compare and other blah else: # update parames according to old_spec
+ return params diff --git a/src/kimchi/vmtemplate.py b/src/kimchi/vmtemplate.py index dd43faa..f92af49 100644 --- a/src/kimchi/vmtemplate.py +++ b/src/kimchi/vmtemplate.py @@ -70,7 +70,7 @@ class VMTemplate(object): # Fetch defaults based on the os distro and version os_distro = args.get('os_distro', iso_distro) os_version = args.get('os_version', iso_version) - name, entry = osinfo.lookup(os_distro, os_version) + entry = osinfo.lookup(os_distro, os_version) self.info.update(entry)
# Override with the passed in parameters diff --git a/tests/test_osinfo.py b/tests/test_osinfo.py index f92567d..da3e1d2 100644 --- a/tests/test_osinfo.py +++ b/tests/test_osinfo.py @@ -25,16 +25,36 @@ from kimchi.osinfo import *
class OSInfoTests(unittest.TestCase): def test_default_lookup(self): - name, entry = lookup(None, None) - self.assertEquals(name, 'unknown') + entry = lookup(None, None) self.assertEquals('unknown', entry['os_distro']) self.assertEquals('unknown', entry['os_version']) self.assertEquals('default', entry['network'])
def test_fedora_lookup(self): cd = 'http://fedora.mirrors.tds.net/pub/fedora/releases/17/Live/x86_64/Fedora-17-x...' - name, entry = lookup('fedora', '17') - self.assertEquals(name, 'fedora') + entry = lookup('fedora', '17') self.assertEquals(10, entry['disks'][0]['size']) self.assertEquals(cd, entry['cdrom']) self.assertEquals('/storagepools/default', entry['storagepool']) + + def test_old_distros(self): + old_versions = {'debian': '5.0', 'ubuntu': '7.04', 'opensuse': '10.1', + 'centos': '5.1', 'rhel': '5.1', 'fedora': '15'} + for distro, version in old_versions.iteritems(): + entry = lookup(distro, version) + self.assertEquals(entry['disk_bus'], 'ide') + self.assertEquals(entry['nic_model'], 'e1000') + + def test_modern_bases(self): + for distro, version in modern_version_bases.iteritems(): + entry = lookup(distro, version) + self.assertEquals(entry['disk_bus'], 'virtio') + self.assertEquals(entry['nic_model'], 'virtio') + + def test_modern_distros(self): + old_versions = {'debian': '7.0', 'ubuntu': '12.04', 'opensuse': '12.3', + 'centos': '6.4', 'rhel': '6.3', 'fedora': '18'} + for distro, version in old_versions.iteritems(): + entry = lookup(distro, version) + self.assertEquals(entry['disk_bus'], 'virtio') + self.assertEquals(entry['nic_model'], 'virtio')
-- Thanks and best regards! Zhou Zheng Sheng / 周征晟 E-mail: zhshzhou@linux.vnet.ibm.com Telephone: 86-10-82454397
We define hard coded template for different distros. It's not flexible to generate templates for other platforms or new distros.
This patch changes to dynamically generate vm template. With this change, it can support new distro, like Fedora20, without any modifications.
Signed-off-by: Mark Wu <wudxw@linux.vnet.ibm.com> --- src/kimchi/osinfo.py | 174 ++++++++++------------------------------------- src/kimchi/vmtemplate.py | 2 +- tests/test_osinfo.py | 28 ++++++-- 3 files changed, 60 insertions(+), 144 deletions(-)
diff --git a/src/kimchi/osinfo.py b/src/kimchi/osinfo.py index 20a93a6..b141d9e 100644 --- a/src/kimchi/osinfo.py +++ b/src/kimchi/osinfo.py @@ -22,136 +22,23 @@
import copy import os +from distutils.version import LooseVersion + + +common_spec = {'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, 'memory': 1024, + 'disks': [{'index': 0, 'size': 10}], 'cdrom_bus': 'ide', + 'cdrom_index': 2} +
On 12/23/2013 10:54 PM, Mark Wu wrote: please, align this dictionary like: { 'cpus:1, 'cpus_cores': 1, ....
+ +modern_spec = dict(common_spec, disk_bus='virtio', nic_model='virtio') + + +old_spec = dict(common_spec, disk_bus='ide', nic_model='e1000') + + +modern_version_bases = {'debian': '6.0', 'ubuntu': '7.10', 'opensuse': '10.3', + 'centos': '5.3', 'rhel': '6.0', 'fedora': '16'}
-osinfo = [ - # Entries are searched in order and the first match will be returned - ('debian', { - 'version': lambda d,v: bool(d == 'debian' and v in ('6.0', '7.0')), - 'icon': 'images/icon-debian.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'virtio', 'nic_model': 'virtio', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('debian-old', { - 'version': lambda d,v: bool(d == 'debian'), - 'icon': 'images/icon-debian.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'ide', 'nic_model': 'e1000', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('ubuntu', { - 'version': lambda d,v: bool(d == 'ubuntu' and v in - ('7.10', '8.04', '8.10', '9.04', '9.10', '10.04', '10.10', - '11.04', '11.10', '12.04', '12.10', '13.04', '13.10')), - 'icon': 'images/icon-ubuntu.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'virtio', 'nic_model': 'virtio', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('ubuntu-old', { - 'version': lambda d,v: bool(d == 'ubuntu'), - 'icon': 'images/icon-ubuntu.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'ide', 'nic_model': 'e1000', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('opensuse', { - 'version': lambda d,v: bool(d == 'opensuse' and v in - ('10.3', '11.0', '11.1', '11.2', '11.3', '11.4', '12.1', '12.2', - '12.3',)), - 'icon': 'images/icon-opensuse.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'virtio', 'nic_model': 'virtio', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('opensuse-old', { - 'version': lambda d,v: bool(d == 'opensuse'), - 'icon': 'images/icon-opensuse.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'ide', 'nic_model': 'e1000', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('fedora', { - 'version': lambda d,v: bool(d == 'fedora' and v in - ('16', '17', '18', '19')), - 'icon': 'images/icon-fedora.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'virtio', 'nic_model': 'virtio', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('fedora-old', { - 'version': lambda d,v: bool(d == 'fedora'), - 'icon': 'images/icon-fedora.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'ide', 'nic_model': 'e1000', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('rhel', { - 'version': lambda d,v: bool(d == 'rhel' and - v.startswith('6.')), - 'icon': 'images/icon-vm.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'virtio', 'nic_model': 'virtio', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('rhel-old', { - 'version': lambda d,v: bool(d == 'rhel'), - 'icon': 'images/icon-vm.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'ide', 'nic_model': 'e1000', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('centos', { - 'version': lambda d,v: bool(d == 'centos' and - (v in ('5.3', '5.4', '5.5') or v.startswith('6.'))), - 'icon': 'images/icon-centos.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'virtio', 'nic_model': 'virtio', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('centos-old', { - 'version': lambda d,v: bool(d == 'centos'), - 'icon': 'images/icon-centos.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'ide', 'nic_model': 'e1000', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('unknown', { - 'version': lambda d,v: True, - 'icon': 'images/icon-vm.png', - 'os_distro': 'unknown', 'os_version': 'unknown', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'cdrom': '', - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'ide', 'nic_model': 'e1000', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), -]
isolinks = { 'debian': { @@ -181,13 +68,22 @@ def lookup(distro, version): 'defaults' and merging the parameters given for the identified OS. If known, a link to a remote install CD is added. """ - for name, entry in osinfo: - # Test if this entry is a valid match - if entry['version'](distro, version): - params = copy.copy(defaults) - params['os_distro'] = distro - params['os_version'] = version - params.update(entry) - params['cdrom'] = isolinks.get(distro, {}).get(version, '') - del params['version'] # Don't pass around the version function - return (name, params) + params = copy.copy(defaults) + params['os_distro'] = distro + params['os_version'] = version + params['cdrom'] = isolinks.get(distro, {}).get(version, '') + + for name, base_version in modern_version_bases.iteritems(): + if name == distro: + params['icon'] = 'images/icon-%s.png' % distro + if LooseVersion(version) >= LooseVersion(base_version): + params.update(modern_spec) + else: + params.update(old_spec) + break + else: + params['icon'] = 'images/icon-vm.png' + params['os_distro'] = params['os_version'] = "unknown" + params.update(old_spec) + + return params diff --git a/src/kimchi/vmtemplate.py b/src/kimchi/vmtemplate.py index dd43faa..f92af49 100644 --- a/src/kimchi/vmtemplate.py +++ b/src/kimchi/vmtemplate.py @@ -70,7 +70,7 @@ class VMTemplate(object): # Fetch defaults based on the os distro and version os_distro = args.get('os_distro', iso_distro) os_version = args.get('os_version', iso_version) - name, entry = osinfo.lookup(os_distro, os_version) + entry = osinfo.lookup(os_distro, os_version) self.info.update(entry)
# Override with the passed in parameters diff --git a/tests/test_osinfo.py b/tests/test_osinfo.py index f92567d..da3e1d2 100644 --- a/tests/test_osinfo.py +++ b/tests/test_osinfo.py @@ -25,16 +25,36 @@ from kimchi.osinfo import *
class OSInfoTests(unittest.TestCase): def test_default_lookup(self): - name, entry = lookup(None, None) - self.assertEquals(name, 'unknown') + entry = lookup(None, None) self.assertEquals('unknown', entry['os_distro']) self.assertEquals('unknown', entry['os_version']) self.assertEquals('default', entry['network'])
def test_fedora_lookup(self): cd = 'http://fedora.mirrors.tds.net/pub/fedora/releases/17/Live/x86_64/Fedora-17-x...' - name, entry = lookup('fedora', '17') - self.assertEquals(name, 'fedora') + entry = lookup('fedora', '17') self.assertEquals(10, entry['disks'][0]['size']) self.assertEquals(cd, entry['cdrom']) self.assertEquals('/storagepools/default', entry['storagepool']) + + def test_old_distros(self): + old_versions = {'debian': '5.0', 'ubuntu': '7.04', 'opensuse': '10.1', + 'centos': '5.1', 'rhel': '5.1', 'fedora': '15'} + for distro, version in old_versions.iteritems(): + entry = lookup(distro, version) + self.assertEquals(entry['disk_bus'], 'ide') + self.assertEquals(entry['nic_model'], 'e1000') + + def test_modern_bases(self): + for distro, version in modern_version_bases.iteritems(): + entry = lookup(distro, version) + self.assertEquals(entry['disk_bus'], 'virtio') + self.assertEquals(entry['nic_model'], 'virtio') + + def test_modern_distros(self): + old_versions = {'debian': '7.0', 'ubuntu': '12.04', 'opensuse': '12.3', + 'centos': '6.4', 'rhel': '6.3', 'fedora': '18'} + for distro, version in old_versions.iteritems(): + entry = lookup(distro, version) + self.assertEquals(entry['disk_bus'], 'virtio') + self.assertEquals(entry['nic_model'], 'virtio')
-- Ramon Nunes Medeiros Software Engineer - Linux Technology Center Brazil IBM Systems & Technology Group Phone : +55 19 2132 7878 ramonn@br.ibm.com
On 12/24/2013 06:41 PM, Ramon Medeiros wrote:
We define hard coded template for different distros. It's not flexible to generate templates for other platforms or new distros.
This patch changes to dynamically generate vm template. With this change, it can support new distro, like Fedora20, without any modifications.
Signed-off-by: Mark Wu <wudxw@linux.vnet.ibm.com> --- src/kimchi/osinfo.py | 174 ++++++++++------------------------------------- src/kimchi/vmtemplate.py | 2 +- tests/test_osinfo.py | 28 ++++++-- 3 files changed, 60 insertions(+), 144 deletions(-)
diff --git a/src/kimchi/osinfo.py b/src/kimchi/osinfo.py index 20a93a6..b141d9e 100644 --- a/src/kimchi/osinfo.py +++ b/src/kimchi/osinfo.py @@ -22,136 +22,23 @@
import copy import os +from distutils.version import LooseVersion + + +common_spec = {'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, 'memory': 1024, + 'disks': [{'index': 0, 'size': 10}], 'cdrom_bus': 'ide', + 'cdrom_index': 2} +
On 12/23/2013 10:54 PM, Mark Wu wrote: please, align this dictionary like:
{ 'cpus:1, 'cpus_cores': 1, .... Ramon, what's the rule do you follow for it?
+ +modern_spec = dict(common_spec, disk_bus='virtio', nic_model='virtio') + + +old_spec = dict(common_spec, disk_bus='ide', nic_model='e1000') + + +modern_version_bases = {'debian': '6.0', 'ubuntu': '7.10', 'opensuse': '10.3', + 'centos': '5.3', 'rhel': '6.0', 'fedora': '16'}
-osinfo = [ - # Entries are searched in order and the first match will be returned - ('debian', { - 'version': lambda d,v: bool(d == 'debian' and v in ('6.0', '7.0')), - 'icon': 'images/icon-debian.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'virtio', 'nic_model': 'virtio', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('debian-old', { - 'version': lambda d,v: bool(d == 'debian'), - 'icon': 'images/icon-debian.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'ide', 'nic_model': 'e1000', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('ubuntu', { - 'version': lambda d,v: bool(d == 'ubuntu' and v in - ('7.10', '8.04', '8.10', '9.04', '9.10', '10.04', '10.10', - '11.04', '11.10', '12.04', '12.10', '13.04', '13.10')), - 'icon': 'images/icon-ubuntu.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'virtio', 'nic_model': 'virtio', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('ubuntu-old', { - 'version': lambda d,v: bool(d == 'ubuntu'), - 'icon': 'images/icon-ubuntu.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'ide', 'nic_model': 'e1000', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('opensuse', { - 'version': lambda d,v: bool(d == 'opensuse' and v in - ('10.3', '11.0', '11.1', '11.2', '11.3', '11.4', '12.1', '12.2', - '12.3',)), - 'icon': 'images/icon-opensuse.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'virtio', 'nic_model': 'virtio', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('opensuse-old', { - 'version': lambda d,v: bool(d == 'opensuse'), - 'icon': 'images/icon-opensuse.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'ide', 'nic_model': 'e1000', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('fedora', { - 'version': lambda d,v: bool(d == 'fedora' and v in - ('16', '17', '18', '19')), - 'icon': 'images/icon-fedora.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'virtio', 'nic_model': 'virtio', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('fedora-old', { - 'version': lambda d,v: bool(d == 'fedora'), - 'icon': 'images/icon-fedora.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'ide', 'nic_model': 'e1000', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('rhel', { - 'version': lambda d,v: bool(d == 'rhel' and - v.startswith('6.')), - 'icon': 'images/icon-vm.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'virtio', 'nic_model': 'virtio', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('rhel-old', { - 'version': lambda d,v: bool(d == 'rhel'), - 'icon': 'images/icon-vm.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'ide', 'nic_model': 'e1000', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('centos', { - 'version': lambda d,v: bool(d == 'centos' and - (v in ('5.3', '5.4', '5.5') or v.startswith('6.'))), - 'icon': 'images/icon-centos.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'virtio', 'nic_model': 'virtio', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('centos-old', { - 'version': lambda d,v: bool(d == 'centos'), - 'icon': 'images/icon-centos.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'ide', 'nic_model': 'e1000', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('unknown', { - 'version': lambda d,v: True, - 'icon': 'images/icon-vm.png', - 'os_distro': 'unknown', 'os_version': 'unknown', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'cdrom': '', - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'ide', 'nic_model': 'e1000', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), -]
isolinks = { 'debian': { @@ -181,13 +68,22 @@ def lookup(distro, version): 'defaults' and merging the parameters given for the identified OS. If known, a link to a remote install CD is added. """ - for name, entry in osinfo: - # Test if this entry is a valid match - if entry['version'](distro, version): - params = copy.copy(defaults) - params['os_distro'] = distro - params['os_version'] = version - params.update(entry) - params['cdrom'] = isolinks.get(distro, {}).get(version, '') - del params['version'] # Don't pass around the version function - return (name, params) + params = copy.copy(defaults) + params['os_distro'] = distro + params['os_version'] = version + params['cdrom'] = isolinks.get(distro, {}).get(version, '') + + for name, base_version in modern_version_bases.iteritems(): + if name == distro: + params['icon'] = 'images/icon-%s.png' % distro + if LooseVersion(version) >= LooseVersion(base_version): + params.update(modern_spec) + else: + params.update(old_spec) + break + else: + params['icon'] = 'images/icon-vm.png' + params['os_distro'] = params['os_version'] = "unknown" + params.update(old_spec) + + return params diff --git a/src/kimchi/vmtemplate.py b/src/kimchi/vmtemplate.py index dd43faa..f92af49 100644 --- a/src/kimchi/vmtemplate.py +++ b/src/kimchi/vmtemplate.py @@ -70,7 +70,7 @@ class VMTemplate(object): # Fetch defaults based on the os distro and version os_distro = args.get('os_distro', iso_distro) os_version = args.get('os_version', iso_version) - name, entry = osinfo.lookup(os_distro, os_version) + entry = osinfo.lookup(os_distro, os_version) self.info.update(entry)
# Override with the passed in parameters diff --git a/tests/test_osinfo.py b/tests/test_osinfo.py index f92567d..da3e1d2 100644 --- a/tests/test_osinfo.py +++ b/tests/test_osinfo.py @@ -25,16 +25,36 @@ from kimchi.osinfo import *
class OSInfoTests(unittest.TestCase): def test_default_lookup(self): - name, entry = lookup(None, None) - self.assertEquals(name, 'unknown') + entry = lookup(None, None) self.assertEquals('unknown', entry['os_distro']) self.assertEquals('unknown', entry['os_version']) self.assertEquals('default', entry['network'])
def test_fedora_lookup(self): cd = 'http://fedora.mirrors.tds.net/pub/fedora/releases/17/Live/x86_64/Fedora-17-x...' - name, entry = lookup('fedora', '17') - self.assertEquals(name, 'fedora') + entry = lookup('fedora', '17') self.assertEquals(10, entry['disks'][0]['size']) self.assertEquals(cd, entry['cdrom']) self.assertEquals('/storagepools/default', entry['storagepool']) + + def test_old_distros(self): + old_versions = {'debian': '5.0', 'ubuntu': '7.04', 'opensuse': '10.1', + 'centos': '5.1', 'rhel': '5.1', 'fedora': '15'} + for distro, version in old_versions.iteritems(): + entry = lookup(distro, version) + self.assertEquals(entry['disk_bus'], 'ide') + self.assertEquals(entry['nic_model'], 'e1000') + + def test_modern_bases(self): + for distro, version in modern_version_bases.iteritems(): + entry = lookup(distro, version) + self.assertEquals(entry['disk_bus'], 'virtio') + self.assertEquals(entry['nic_model'], 'virtio') + + def test_modern_distros(self): + old_versions = {'debian': '7.0', 'ubuntu': '12.04', 'opensuse': '12.3', + 'centos': '6.4', 'rhel': '6.3', 'fedora': '18'} + for distro, version in old_versions.iteritems(): + entry = lookup(distro, version) + self.assertEquals(entry['disk_bus'], 'virtio') + self.assertEquals(entry['nic_model'], 'virtio')
On 12/23/2013 10:54 PM, Mark Wu wrote:
We define hard coded template for different distros. It's not flexible to generate templates for other platforms or new distros.
This patch changes to dynamically generate vm template. With this change, it can support new distro, like Fedora20, without any modifications.
Signed-off-by: Mark Wu <wudxw@linux.vnet.ibm.com> --- src/kimchi/osinfo.py | 174 ++++++++++------------------------------------- src/kimchi/vmtemplate.py | 2 +- tests/test_osinfo.py | 28 ++++++-- 3 files changed, 60 insertions(+), 144 deletions(-)
diff --git a/src/kimchi/osinfo.py b/src/kimchi/osinfo.py index 20a93a6..b141d9e 100644 --- a/src/kimchi/osinfo.py +++ b/src/kimchi/osinfo.py @@ -22,136 +22,23 @@
import copy import os
2 lines
+from distutils.version import LooseVersion + + +common_spec = {'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, 'memory': 1024, + 'disks': [{'index': 0, 'size': 10}], 'cdrom_bus': 'ide', + 'cdrom_index': 2} + + +modern_spec = dict(common_spec, disk_bus='virtio', nic_model='virtio') + + +old_spec = dict(common_spec, disk_bus='ide', nic_model='e1000') + + +modern_version_bases = {'debian': '6.0', 'ubuntu': '7.10', 'opensuse': '10.3', + 'centos': '5.3', 'rhel': '6.0', 'fedora': '16'}
-osinfo = [ - # Entries are searched in order and the first match will be returned - ('debian', { - 'version': lambda d,v: bool(d == 'debian' and v in ('6.0', '7.0')), - 'icon': 'images/icon-debian.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'virtio', 'nic_model': 'virtio', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('debian-old', { - 'version': lambda d,v: bool(d == 'debian'), - 'icon': 'images/icon-debian.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'ide', 'nic_model': 'e1000', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('ubuntu', { - 'version': lambda d,v: bool(d == 'ubuntu' and v in - ('7.10', '8.04', '8.10', '9.04', '9.10', '10.04', '10.10', - '11.04', '11.10', '12.04', '12.10', '13.04', '13.10')), - 'icon': 'images/icon-ubuntu.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'virtio', 'nic_model': 'virtio', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('ubuntu-old', { - 'version': lambda d,v: bool(d == 'ubuntu'), - 'icon': 'images/icon-ubuntu.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'ide', 'nic_model': 'e1000', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('opensuse', { - 'version': lambda d,v: bool(d == 'opensuse' and v in - ('10.3', '11.0', '11.1', '11.2', '11.3', '11.4', '12.1', '12.2', - '12.3',)), - 'icon': 'images/icon-opensuse.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'virtio', 'nic_model': 'virtio', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('opensuse-old', { - 'version': lambda d,v: bool(d == 'opensuse'), - 'icon': 'images/icon-opensuse.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'ide', 'nic_model': 'e1000', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('fedora', { - 'version': lambda d,v: bool(d == 'fedora' and v in - ('16', '17', '18', '19')), - 'icon': 'images/icon-fedora.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'virtio', 'nic_model': 'virtio', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('fedora-old', { - 'version': lambda d,v: bool(d == 'fedora'), - 'icon': 'images/icon-fedora.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'ide', 'nic_model': 'e1000', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('rhel', { - 'version': lambda d,v: bool(d == 'rhel' and - v.startswith('6.')), - 'icon': 'images/icon-vm.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'virtio', 'nic_model': 'virtio', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('rhel-old', { - 'version': lambda d,v: bool(d == 'rhel'), - 'icon': 'images/icon-vm.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'ide', 'nic_model': 'e1000', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('centos', { - 'version': lambda d,v: bool(d == 'centos' and - (v in ('5.3', '5.4', '5.5') or v.startswith('6.'))), - 'icon': 'images/icon-centos.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'virtio', 'nic_model': 'virtio', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('centos-old', { - 'version': lambda d,v: bool(d == 'centos'), - 'icon': 'images/icon-centos.png', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'ide', 'nic_model': 'e1000', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), - ('unknown', { - 'version': lambda d,v: True, - 'icon': 'images/icon-vm.png', - 'os_distro': 'unknown', 'os_version': 'unknown', - 'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, - 'memory': 1024, - 'cdrom': '', - 'disks': [{'index': 0, 'size': 10}], - 'disk_bus': 'ide', 'nic_model': 'e1000', - 'cdrom_bus': 'ide', 'cdrom_index': 2, - }), -]
isolinks = { 'debian': { @@ -181,13 +68,22 @@ def lookup(distro, version): 'defaults' and merging the parameters given for the identified OS. If known, a link to a remote install CD is added. """ - for name, entry in osinfo: - # Test if this entry is a valid match - if entry['version'](distro, version): - params = copy.copy(defaults) - params['os_distro'] = distro - params['os_version'] = version - params.update(entry) - params['cdrom'] = isolinks.get(distro, {}).get(version, '') - del params['version'] # Don't pass around the version function - return (name, params) + params = copy.copy(defaults) + params['os_distro'] = distro + params['os_version'] = version + params['cdrom'] = isolinks.get(distro, {}).get(version, '') +
+ for name, base_version in modern_version_bases.iteritems(): + if name == distro: + params['icon'] = 'images/icon-%s.png' % distro + if LooseVersion(version) >= LooseVersion(base_version): + params.update(modern_spec) + else: + params.update(old_spec) + break
Same as commented by Zhou Zheng Sheng You can access the dict directly. base_version = modern_version_bases.get(distro)
+ else: + params['icon'] = 'images/icon-vm.png' + params['os_distro'] = params['os_version'] = "unknown" + params.update(old_spec) + + return params diff --git a/src/kimchi/vmtemplate.py b/src/kimchi/vmtemplate.py index dd43faa..f92af49 100644 --- a/src/kimchi/vmtemplate.py +++ b/src/kimchi/vmtemplate.py @@ -70,7 +70,7 @@ class VMTemplate(object): # Fetch defaults based on the os distro and version os_distro = args.get('os_distro', iso_distro) os_version = args.get('os_version', iso_version) - name, entry = osinfo.lookup(os_distro, os_version) + entry = osinfo.lookup(os_distro, os_version) self.info.update(entry)
# Override with the passed in parameters diff --git a/tests/test_osinfo.py b/tests/test_osinfo.py index f92567d..da3e1d2 100644 --- a/tests/test_osinfo.py +++ b/tests/test_osinfo.py @@ -25,16 +25,36 @@ from kimchi.osinfo import *
class OSInfoTests(unittest.TestCase): def test_default_lookup(self): - name, entry = lookup(None, None) - self.assertEquals(name, 'unknown') + entry = lookup(None, None) self.assertEquals('unknown', entry['os_distro']) self.assertEquals('unknown', entry['os_version']) self.assertEquals('default', entry['network'])
def test_fedora_lookup(self): cd = 'http://fedora.mirrors.tds.net/pub/fedora/releases/17/Live/x86_64/Fedora-17-x...' - name, entry = lookup('fedora', '17') - self.assertEquals(name, 'fedora') + entry = lookup('fedora', '17') self.assertEquals(10, entry['disks'][0]['size']) self.assertEquals(cd, entry['cdrom']) self.assertEquals('/storagepools/default', entry['storagepool']) + + def test_old_distros(self): + old_versions = {'debian': '5.0', 'ubuntu': '7.04', 'opensuse': '10.1', + 'centos': '5.1', 'rhel': '5.1', 'fedora': '15'} + for distro, version in old_versions.iteritems(): + entry = lookup(distro, version) + self.assertEquals(entry['disk_bus'], 'ide') + self.assertEquals(entry['nic_model'], 'e1000') + + def test_modern_bases(self): + for distro, version in modern_version_bases.iteritems(): + entry = lookup(distro, version) + self.assertEquals(entry['disk_bus'], 'virtio') + self.assertEquals(entry['nic_model'], 'virtio') + + def test_modern_distros(self): + old_versions = {'debian': '7.0', 'ubuntu': '12.04', 'opensuse': '12.3', + 'centos': '6.4', 'rhel': '6.3', 'fedora': '18'} + for distro, version in old_versions.iteritems(): + entry = lookup(distro, version) + self.assertEquals(entry['disk_bus'], 'virtio') + self.assertEquals(entry['nic_model'], 'virtio')
participants (4)
-
Aline Manera -
Mark Wu -
Ramon Medeiros -
Zhou Zheng Sheng