June 2016 - Kimchi-devel - oVirt List Archives

[PATCH][Kimchi] Do not use default value when declare a function
by Ramon Medeiros 08 Jun '16

08 Jun '16

For mutable object as a default parameter in function and method-declarations the problem is, that the evaluation and creation takes place at exactly the same moment. The python-parser reads the function-head and evaluates it at the same moment. Most beginers assume that a new object is created at every call, but that's not correct. One object (in your example a list) is created at the moment of declaration and not on demand when you are calling the method. For imutable objects that's not a problem, because even if all calls share the same object, it's imutable and therefore it's properties remain the same. As a convention, must be used the None object for defaults to indicate the use of a default initialization, which now can take place in the function-body, which naturally is evaluated at call-time. Signed-off-by: Ramon Medeiros <ramonn(a)linux.vnet.ibm.com> --- mockmodel.py | 5 ++++- model/networks.py | 5 ++++- model/templates.py | 5 ++++- model/vmsnapshots.py | 4 +++- network.py | 5 ++++- xmlutils/bootorder.py | 5 ++++- xmlutils/cpu.py | 4 +++- 7 files changed, 26 insertions(+), 7 deletions(-) diff --git a/mockmodel.py b/mockmodel.py index 6c70472..9f9de52 100644 --- a/mockmodel.py +++ b/mockmodel.py @@ -714,7 +714,10 @@ class MockDevices(object): class MockVMSnapshot(object): - def __init__(self, name, params={}): + def __init__(self, name, params=None): + if params is None: + params = {} + self.name = name self.current = True diff --git a/model/networks.py b/model/networks.py index 3b2a9e2..4b722d3 100644 --- a/model/networks.py +++ b/model/networks.py @@ -123,7 +123,10 @@ class NetworksModel(object): names = conn.listNetworks() + conn.listDefinedNetworks() return sorted(map(lambda x: x.decode('utf-8'), names)) - def _get_available_address(self, addr_pools=[]): + def _get_available_address(self, addr_pools=None): + if addr_pools is None: + addr_pools = [] + invalid_addrs = [] for net_name in self.get_list(): network = NetworkModel.get_network(self.conn.get(), net_name) diff --git a/model/templates.py b/model/templates.py index 25bc017..a299c85 100644 --- a/model/templates.py +++ b/model/templates.py @@ -177,7 +177,10 @@ class TemplateModel(object): self.templates = TemplatesModel(**kargs) @staticmethod - def get_template(name, objstore, conn, overrides={}): + def get_template(name, objstore, conn, overrides=None): + if overrides is None: + overrides = {} + with objstore as session: params = session.get('template', name) if overrides and 'storagepool' in overrides: diff --git a/model/vmsnapshots.py b/model/vmsnapshots.py index fd17d2f..6f2483c 100644 --- a/model/vmsnapshots.py +++ b/model/vmsnapshots.py @@ -40,7 +40,7 @@ class VMSnapshotsModel(object): self.vmstorages = VMStoragesModel(**kargs) self.vmstorage = VMStorageModel(**kargs) - def create(self, vm_name, params={}): + def create(self, vm_name, params=None): """Create a snapshot with the current domain state. The VM must be stopped and contain only disks with format 'qcow2'; @@ -55,6 +55,8 @@ class VMSnapshotsModel(object): Return: A Task running the operation. """ + if params is None: + params = {} vir_dom = VMModel.get_vm(vm_name, self.conn) if DOM_STATE_MAP[vir_dom.info()[0]] != u'shutoff': raise InvalidOperation('KCHSNAP0001E', {'vm': vm_name}) diff --git a/network.py b/network.py index dd7b50f..ec567ca 100644 --- a/network.py +++ b/network.py @@ -52,7 +52,10 @@ def get_dev_netaddrs(): # used_nets should include all the subnet allocated in libvirt network # will get host network by get_dev_netaddrs -def get_one_free_network(used_nets, nets_pool=PrivateNets): +def get_one_free_network(used_nets, nets_pool=None): + if nets_pool is None: + nets_pool = PrivateNets + def _get_free_network(nets, used_nets): for net in nets.subnet(new_prefix=24): if not any(net.overlaps(used) for used in used_nets): diff --git a/xmlutils/bootorder.py b/xmlutils/bootorder.py index 28457e7..9e444bd 100644 --- a/xmlutils/bootorder.py +++ b/xmlutils/bootorder.py @@ -21,7 +21,7 @@ import lxml.etree as ET from lxml.builder import E -def get_bootorder_xml(boot_order=['hd', 'cdrom', 'network']): +def get_bootorder_xml(boot_order=None): """ Returns the XML for boot order. The default return includes the following: @@ -31,6 +31,9 @@ def get_bootorder_xml(boot_order=['hd', 'cdrom', 'network']): To a different boot order, specify the order by a list as argument. """ + if boot_order is None: + boot_order = ['hd', 'cdrom', 'network'] + boot_xml = '' for device in boot_order: boot = E.boot(dev=device) diff --git a/xmlutils/cpu.py b/xmlutils/cpu.py index 08a6afb..7548be2 100644 --- a/xmlutils/cpu.py +++ b/xmlutils/cpu.py @@ -45,7 +45,7 @@ def get_topology_xml(cpu_topo): return ET.tostring(xml) -def get_cpu_xml(cpus, memory, cpu_topo={}): +def get_cpu_xml(cpus, memory, cpu_topo=None): # Returns the libvirt CPU element based on given numa and topology # CPU element will always have numa element # <cpu> @@ -54,6 +54,8 @@ def get_cpu_xml(cpus, memory, cpu_topo={}): # </numa> # <topology sockets='1' cores='2' threads='1'/> # </cpu> + if cpu_topo is None: + cpu_topo = {} xml = E.cpu(ET.fromstring(get_numa_xml(cpus, memory))) if cpu_topo: xml.insert(0, ET.fromstring(get_topology_xml(cpu_topo))) -- 2.5.5

3 2

Re: [Kimchi-devel] Clean FEATURETEST_VM effectively.
by Aline Manera 08 Jun '16

08 Jun '16

Applied. Thanks. Regards, Aline Manera

1 0

Re: [Kimchi-devel] [PATCH][Kimchi] Substitute quotes by apostrophe in configuration file
by Aline Manera 08 Jun '16

08 Jun '16

Applied. Thanks. Regards, Aline Manera

1 0

Re: [Kimchi-devel] [PATCH] [Wok 1/2] Include user IP address in request log messages
by Aline Manera 08 Jun '16

08 Jun '16

Hi Lucio, The patch looks good! But please, update API.md documentation and if possible, add/change test cases to confirm the API keeps working as expected. Thanks, Aline Manera On 06/03/2016 06:40 PM, Lucio Correia wrote: > * Allow advanced log searches by IP address > * Add RECORD_TEMPLATE_DICT to avoid versioning errors > with older log entries, which don't have IP address. > * Remove duplicated comment line > > Signed-off-by: Lucio Correia <luciojhc(a)linux.vnet.ibm.com> > --- > src/wok/control/base.py | 10 ++++++---- > src/wok/reqlogger.py | 17 ++++++++++++++--- > src/wok/root.py | 6 ++++-- > 3 files changed, 24 insertions(+), 9 deletions(-) > > diff --git a/src/wok/control/base.py b/src/wok/control/base.py > index 4b459ed..419179a 100644 > --- a/src/wok/control/base.py > +++ b/src/wok/control/base.py > @@ -146,7 +146,8 @@ class Resource(object): > msg, > app=get_plugin_from_request(), > req=method, > - user=cherrypy.session.get(USER_NAME, 'N/A') > + user=cherrypy.session.get(USER_NAME, 'N/A'), > + ip=cherrypy.request.remote.ip > ).log() > > if destructive is False or \ > @@ -227,7 +228,8 @@ class Resource(object): > msg, > app=get_plugin_from_request(), > req=method, > - user=cherrypy.session.get(USER_NAME, 'N/A') > + user=cherrypy.session.get(USER_NAME, 'N/A'), > + ip=cherrypy.request.remote.ip > ).log() > > return result > @@ -374,7 +376,6 @@ class Collection(object): > except Exception as e: > # In case of errors when fetching a resource info, pass and > # log the error, so, other resources are returned > - # log the error, so, other resources are returned. > # Encoding error message as ident is also encoded value. > # This has to be done to avoid unicode error, > # as combination of encoded and unicode value results into > @@ -455,7 +456,8 @@ class Collection(object): > msg, > app=get_plugin_from_request(), > req=method, > - user=cherrypy.session.get(USER_NAME, 'N/A') > + user=cherrypy.session.get(USER_NAME, 'N/A'), > + ip=cherrypy.request.remote.ip > ).log() > > return result > diff --git a/src/wok/reqlogger.py b/src/wok/reqlogger.py > index bbd1bd0..2ec62b8 100644 > --- a/src/wok/reqlogger.py > +++ b/src/wok/reqlogger.py > @@ -34,10 +34,20 @@ from wok.utils import ascii_dict, remove_old_files > > > # Log search setup > -FILTER_FIELDS = ['app', 'date', 'download', 'req', 'user', 'time'] > +FILTER_FIELDS = ['app', 'date', 'download', 'ip', 'req', 'user', 'time'] > LOG_DOWNLOAD_URI = "/data/logs/%s" > LOG_DOWNLOAD_TIMEOUT = 6 > -LOG_FORMAT = "[%(date)s %(time)s] %(req)-6s %(app)-11s %(user)s: %(message)s\n" > +LOG_FORMAT = "[%(date)s %(time)s] %(req)-6s %(app)-11s %(ip)-15s %(user)s: " \ > + "%(message)s\n" > +RECORD_TEMPLATE_DICT = { > + 'date': '', > + 'time': '', > + 'req': '', > + 'app': '', > + 'ip': '', > + 'user': '', > + 'message': '', > +} > SECONDS_PER_HOUR = 360 > > # Log handler setup > @@ -86,7 +96,8 @@ class RequestParser(object): > > with fd: > for record in sortedList: > - asciiRecord = ascii_dict(record) > + asciiRecord = RECORD_TEMPLATE_DICT > + asciiRecord.update(ascii_dict(record)) > fd.write(LOG_FORMAT % asciiRecord) > > fd.close() > diff --git a/src/wok/root.py b/src/wok/root.py > index 29ea657..902e203 100644 > --- a/src/wok/root.py > +++ b/src/wok/root.py > @@ -172,7 +172,8 @@ class WokRoot(Root): > msg, > app=get_plugin_from_request(), > req=method, > - user=cherrypy.session.get(auth.USER_NAME, 'N/A') > + user=cherrypy.session.get(auth.USER_NAME, 'N/A'), > + ip=cherrypy.request.remote.ip > ).log() > > return json.dumps(user_info) > @@ -187,7 +188,8 @@ class WokRoot(Root): > msg, > app=get_plugin_from_request(), > req=method, > - user=params['username'] > + user=params['username'], > + ip=cherrypy.request.remote.ip > ).log() > > auth.logout()

1 0

[PATCH][Wok] Do not use default value when declare a function
by Ramon Medeiros 08 Jun '16

08 Jun '16

For mutable object as a default parameter in function and method-declarations the problem is, that the evaluation and creation takes place at exactly the same moment. The python-parser reads the function-head and evaluates it at the same moment. Most beginers assume that a new object is created at every call, but that's not correct. One object (in your example a list) is created at the moment of declaration and not on demand when you are calling the method. For imutable objects that's not a problem, because even if all calls share the same object, it's imutable and therefore it's properties remain the same. As a convention, must be used the None object for defaults to indicate the use of a default initialization, which now can take place in the function-body, which naturally is evaluated at call-time. Signed-off-by: Ramon Medeiros <ramonn(a)linux.vnet.ibm.com> --- src/wok/exception.py | 4 +++- src/wok/message.py | 4 +++- src/wok/model/notifications.py | 4 +++- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/wok/exception.py b/src/wok/exception.py index 52f007e..90b6410 100644 --- a/src/wok/exception.py +++ b/src/wok/exception.py @@ -25,7 +25,9 @@ from wok.message import WokMessage class WokException(Exception): - def __init__(self, code='', args={}): + def __init__(self, code='', args=None): + if args is None: + args = {} self.code = code msg = WokMessage(code, args).get_text() cherrypy.log.error_log.error(msg) diff --git a/src/wok/message.py b/src/wok/message.py index a7162c7..4c35747 100644 --- a/src/wok/message.py +++ b/src/wok/message.py @@ -26,7 +26,9 @@ from wok.template import get_lang, validate_language class WokMessage(object): - def __init__(self, code='', args={}, plugin=None): + def __init__(self, code='', args=None, plugin=None): + if args is None: + args = {} # make all args unicode for key, value in args.iteritems(): if isinstance(value, unicode): diff --git a/src/wok/model/notifications.py b/src/wok/model/notifications.py index e83ac56..bdb7c78 100644 --- a/src/wok/model/notifications.py +++ b/src/wok/model/notifications.py @@ -27,7 +27,9 @@ from wok.utils import wok_log notificationsStore = {} -def add_notification(code, args={}, plugin_name=None): +def add_notification(code, args=None, plugin_name=None): + if args is None: + args = {} if not code: wok_log.error("Unable to add notification: invalid code '%(code)s'" % {'code': str(code)}) -- 2.5.5

3 2

[PATCH] [Kimchi] Clean FEATURETEST_VM effectively.
by pvital＠linux.vnet.ibm.com 07 Jun '16

07 Jun '16

From: Paulo Vital <pvital(a)linux.vnet.ibm.com> FEATURETEST_VM is defined at Kimchi start up to check if few QEMU and Libvirt features are enabled in the host system. Some bugs reported the existence of this VM running in the system, even after the Kimchi Feature Tests have completed. This patch adds a cleaning step before start the Kimchi Feature Tests. In addition, modified the has_mem_hotplug_support() to not use RollbackContext and guarantee that FEATURETEST_VM will be undefined. Signed-off-by: Paulo Vital <pvital(a)linux.vnet.ibm.com> --- model/config.py | 3 +++ model/featuretests.py | 33 +++++++++++++++++---------------- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/model/config.py b/model/config.py index fc4b285..78cdaeb 100644 --- a/model/config.py +++ b/model/config.py @@ -57,6 +57,9 @@ class CapabilitiesModel(object): self.mem_hotplug_support = False self.libvirtd_running = False + # make sure there're no Kimchi leftovers from previous executions + self._clean_leftovers() + # run feature tests self._set_capabilities() diff --git a/model/featuretests.py b/model/featuretests.py index 2b680c3..fca152d 100644 --- a/model/featuretests.py +++ b/model/featuretests.py @@ -24,7 +24,6 @@ import platform import subprocess from lxml.builder import E -from wok.rollbackcontext import RollbackContext from wok.utils import run_command, servermethod, wok_log @@ -220,22 +219,24 @@ class FeatureTests(object): # Libvirt < 1.2.14 does not support memory devices, so try to attach a # device. Then check if QEMU (>= 2.1) supports memory hotplug, starting # the guest These steps avoid errors with Libvirt 'test' driver for KVM - with RollbackContext() as rollback: + conn_type = conn.getType().lower() + domain_type = 'test' if conn_type == 'test' else 'kvm' + arch = 'i686' if conn_type == 'test' else platform.machine() + arch = 'ppc64' if arch == 'ppc64le' else arch + + dom = None + try: FeatureTests.disable_libvirt_error_logging() - rollback.prependDefer(FeatureTests.enable_libvirt_error_logging) - conn_type = conn.getType().lower() - domain_type = 'test' if conn_type == 'test' else 'kvm' - arch = 'i686' if conn_type == 'test' else platform.machine() - arch = 'ppc64' if arch == 'ppc64le' else arch dom = conn.defineXML(MAXMEM_VM_XML % {'name': FEATURETEST_VM_NAME, 'domain': domain_type, 'arch': arch}) - rollback.prependDefer(dom.undefine) - try: - dom.attachDeviceFlags(DEV_MEM_XML, - libvirt.VIR_DOMAIN_MEM_CONFIG) - dom.create() - rollback.prependDefer(dom.destroy) - return True - except libvirt.libvirtError: - return False + dom.attachDeviceFlags(DEV_MEM_XML, libvirt.VIR_DOMAIN_MEM_CONFIG) + dom.create() + except libvirt.libvirtError: + return False + finally: + if (dom and dom.isActive() == 1): + dom.destroy() + dom is None or dom.undefine() + FeatureTests.enable_libvirt_error_logging() + return True -- 2.5.5

2 1

[PATCH][Wok] Warn user when server is not reachable at login page
by Ramon Medeiros 07 Jun '16

07 Jun '16

Signed-off-by: Ramon Medeiros <ramonn(a)linux.vnet.ibm.com> --- ui/js/src/wok.login.js | 10 ++++++++-- ui/pages/login.html.tmpl | 1 + 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/ui/js/src/wok.login.js b/ui/js/src/wok.login.js index 7423f81..0b09508 100644 --- a/ui/js/src/wok.login.js +++ b/ui/js/src/wok.login.js @@ -74,8 +74,14 @@ wok.login_main = function() { } wok.cookie.set('roles',JSON.stringify(data.roles)); window.location.replace(window.location.pathname.replace(/\/+login.html/, '') + next_url); - }, function() { - $("#messUserPass").show(); + }, function(jqXHR, textStatus, errorThrown) { + if (jqXHR.responseText == "") { + $("#messUserPass").hide(); + $("#missServer").show(); + } else { + $("#missServer").hide(); + $("#messUserPass").show(); + } $("#messSession").hide(); $("#logging").hide(); $("#login").show(); diff --git a/ui/pages/login.html.tmpl b/ui/pages/login.html.tmpl index 0357d64..d6daeb7 100644 --- a/ui/pages/login.html.tmpl +++ b/ui/pages/login.html.tmpl @@ -101,6 +101,7 @@ <div class="err-area"> <div id="messUserPass" class="alert alert-danger" style="display: none;">$_("The username or password you entered is incorrect. Please try again.")</div> <div id="messSession" class="alert alert-danger" style="display: none;">$_("Session timeout, please re-login.")</div> + <div id="missServer" class="alert alert-danger" style="display: none;">$_("Server unreachable")</div> </div> <form id="form-login" class="form-horizontal" method="post"> <div class="form-group"> -- 2.5.5

2 1

[PATCH][Kimchi 5/5] Add test to check bootorder
by Ramon Medeiros 07 Jun '16

07 Jun '16

Signed-off-by: Ramon Medeiros <ramonn(a)linux.vnet.ibm.com> --- tests/test_model.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/test_model.py b/tests/test_model.py index f4a145f..04c20df 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -1125,6 +1125,11 @@ class ModelTests(unittest.TestCase): self.assertEquals([], inst.vm_lookup(u'пeω-∨м')['users']) self.assertEquals([], inst.vm_lookup(u'пeω-∨м')['groups']) + # change bootorder + b_order = ["hd", "network", "cdrom"] + inst.vm_update(u'пeω-∨м', {"boot":{"order":b_order}}) + self.assertEquals(b_order, inst.vm_lookup(u'пeω-∨м')['boot']) + def test_get_interfaces(self): inst = model.Model('test:///default', objstore_loc=self.tmp_store) -- 2.5.5

2 1

Re: [Kimchi-devel] [PATCH][Kimchi 0/5] Make Kimchi able to change guest's boot order
by Ramon Medeiros 07 Jun '16

07 Jun '16

How to use: curl -u root -H "Content-Type: application/json" -H "Accept: application/json" -k http://localhost:8010/plugins/kimchi/vms/fedora -X PUT -d '{"boot":{"order":["cdrom", "hd"]}}' Enter host password for user 'root': { "users":[], "screenshot":null, "persistent":true, "cpu_info":{ "maxvcpus":1, "vcpus":1, "topology":{} }, "groups":[], "graphics":{ "passwd":null, "passwdValidTo":null, "type":"vnc", "port":null, "listen":"127.0.0.1" }, "icon":"plugins/kimchi/images/icon-fedora.png", "stats":{ "cpu_utilization":0, "io_throughput":0, "io_throughput_peak":100, "net_throughput":0, "mem_utilization":0, "net_throughput_peak":100 }, "name":"fedora", "boot":[ "cdrom", "hd" ], "uuid":"98a82c16-c6ae-4253-b018-08ab3fde8f5e", "access":"full", "state":"shutoff", "memory":{ "current":1024, "maxmemory":1024 } } On 06/06/2016 06:13 PM, Ramon Medeiros wrote: > Ramon Medeiros (5): > Create method to change bootorder of a guest > Update documentation about bootorder on vm update > Update REST API > Add function to retrieve bootorder on vm lookup > Add test to check bootorder > > API.json | 12 ++++++++++++ > docs/API.md | 2 ++ > i18n.py | 2 ++ > model/vms.py | 41 +++++++++++++++++++++++++++++++++++++++-- > tests/test_model.py | 5 +++++ > 5 files changed, 60 insertions(+), 2 deletions(-) > -- Ramon Nunes Medeiros Kimchi Developer Linux Technology Center Brazil IBM Systems & Technology Group Phone : +55 19 2132 7878 ramonn(a)br.ibm.com

2 1

[PATCH][Kimchi 4/5] Add function to retrieve bootorder on vm lookup
by Ramon Medeiros 07 Jun '16

07 Jun '16

Signed-off-by: Ramon Medeiros <ramonn(a)linux.vnet.ibm.com> --- model/vms.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/model/vms.py b/model/vms.py index 854cd04..ad788bc 100644 --- a/model/vms.py +++ b/model/vms.py @@ -94,6 +94,7 @@ XPATH_DOMAIN_MEMORY_UNIT = '/domain/memory/@unit' XPATH_DOMAIN_UUID = '/domain/uuid' XPATH_DOMAIN_DEV_CPU_ID = '/domain/devices/spapr-cpu-socket/@id' +XPATH_BOOT = 'os/boot/@dev' XPATH_CPU = './cpu' XPATH_NAME = './name' XPATH_NUMA_CELL = './cpu/numa/cell' @@ -1250,6 +1251,9 @@ class VMModel(object): else: maxmemory = memory + # get boot order + boot = xpath_get_text(xml, XPATH_BOOT) + return {'name': name, 'state': state, 'stats': res, @@ -1267,7 +1271,8 @@ class VMModel(object): 'users': users, 'groups': groups, 'access': 'full', - 'persistent': True if dom.isPersistent() else False + 'persistent': True if dom.isPersistent() else False, + 'boot': boot } def _vm_get_disk_paths(self, dom): -- 2.5.5

1 0