Re: [Kimchi-devel] [PATCH][Kimchi] Do not use default value when declare a function

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&gt; --- 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)))