From b27c4caa492efbfbd211ebe34087ebaba03ae3f7 Mon Sep 17 00:00:00 2001 From: Bojan Vitnik Date: Mon, 8 Apr 2019 18:27:02 +0200 Subject: [PATCH] XenServer: Minor bug fixes 2 (#54697) - xenserver module_util: fixed comment in set_vm_power_state(). - xenserver module_util: renamed cdrom.iso VM fact to cdrom.iso_name in gather_vm_facts() to be in line with module parameter of same name. - xenserver module_util: sorted IPv6 addresses by their OS index in gather_vm_facts(). - xenserver_guest module: fixed a bug in deploy() where an error message would not be shown when VM name is empty and check mode is used. - xenserver_guest module: fixed a bug in destroy() where VM would be shut down by error when check mode is used. - xenserver_guest module: fixed a bug in get_changes() where wrong out-of-bound disk position would be shown in error message when CD-ROM device occupies last position. - xenserver_guest module: assume value "none" for "networks.type" and "networks.type6" module parameters in get_changes() when no value is found in xenstore_data (custom customization agent). - xenserver_guest module: added separate error message in get_changes() for a case when maximum number of network interfaces is reached. - xenserver_guest module: negative value for disk size in get_normalized_disk_size() now properly shows an error. --- lib/ansible/module_utils/xenserver.py | 7 ++-- .../cloud/xenserver/xenserver_guest.py | 33 +++++++++++-------- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/lib/ansible/module_utils/xenserver.py b/lib/ansible/module_utils/xenserver.py index f09ef521973..ee2c9c982a0 100644 --- a/lib/ansible/module_utils/xenserver.py +++ b/lib/ansible/module_utils/xenserver.py @@ -478,7 +478,7 @@ def gather_vm_facts(module, vm_params): vm_facts['cdrom'].update(type="none") else: vm_facts['cdrom'].update(type="iso") - vm_facts['cdrom'].update(iso=vm_vbd_params['VDI']['name_label']) + vm_facts['cdrom'].update(iso_name=vm_vbd_params['VDI']['name_label']) for vm_vif_params in vm_params['VIFs']: vm_guest_metrics_networks = vm_params['guest_metrics'].get('networks', {}) @@ -492,7 +492,8 @@ def gather_vm_facts(module, vm_params): "prefix": "", "netmask": "", "gateway": "", - "ip6": [vm_guest_metrics_networks[ipv6] for ipv6 in vm_guest_metrics_networks.keys() if ipv6.startswith("%s/ipv6/" % vm_vif_params['device'])], + "ip6": [vm_guest_metrics_networks[ipv6] for ipv6 in sorted(vm_guest_metrics_networks.keys()) if ipv6.startswith("%s/ipv6/" % + vm_vif_params['device'])], "prefix6": "", "gateway6": "", } @@ -575,7 +576,7 @@ def set_vm_power_state(module, vm_ref, power_state, timeout=300): # hard_shutdown will halt VM regardless of current state. xapi_session.xenapi.VM.hard_shutdown(vm_ref) elif power_state == "restarted": - # hard_restart will restart VM only if VM is in paused or running state. + # hard_reboot will restart VM only if VM is in paused or running state. if vm_power_state_current in ["paused", "poweredon"]: if not module.check_mode: xapi_session.xenapi.VM.hard_reboot(vm_ref) diff --git a/lib/ansible/modules/cloud/xenserver/xenserver_guest.py b/lib/ansible/modules/cloud/xenserver/xenserver_guest.py index 3387fd3c43a..eaaabdf1ace 100644 --- a/lib/ansible/modules/cloud/xenserver/xenserver_guest.py +++ b/lib/ansible/modules/cloud/xenserver/xenserver_guest.py @@ -528,14 +528,14 @@ class XenServerVM(XenServerObject): else: self.module.fail_json(msg="VM deploy disks[0]: no default SR found! You must specify SR explicitly.") - # Support for Ansible check mode. - if self.module.check_mode: - return - # VM name could be an empty string which is bad. if self.module.params['name'] is not None and not self.module.params['name']: self.module.fail_json(msg="VM deploy: VM name must not be an empty string!") + # Support for Ansible check mode. + if self.module.check_mode: + return + # Now we can instantiate VM. We use VM.clone for linked_clone and # VM.copy for non linked_clone. if self.module.params['linked_clone']: @@ -1030,16 +1030,16 @@ class XenServerVM(XenServerObject): if not self.exists(): self.module.fail_json(msg="Called destroy on non existing VM!") - if self.vm_params['power_state'].lower() != 'halted': - if self.module.params['force']: - self.set_power_state("poweredoff") - else: - self.module.fail_json(msg="VM destroy: VM has to be in powered off state to destroy but force was not specified!") + if self.vm_params['power_state'].lower() != 'halted' and not self.module.params['force']: + self.module.fail_json(msg="VM destroy: VM has to be in powered off state to destroy but force was not specified!") # Support for Ansible check mode. if self.module.check_mode: return + # Make sure that VM is poweredoff before we can destroy it. + self.set_power_state("poweredoff") + try: # Destroy VM! self.xapi_session.xenapi.VM.destroy(self.vm_ref) @@ -1295,8 +1295,12 @@ class XenServerVM(XenServerObject): vm_disk_userdevice_highest = userdevice break + # If no place was found. if disk_userdevice is None: - disk_userdevice = str(int(vm_disk_userdevice_highest) + 1) + # Highest occupied place could be a CD-ROM device + # so we have to include all devices regardless of + # type when calculating out-of-bound position. + disk_userdevice = str(int(self.vm_params['VBDs'][-1]['userdevice']) + 1) self.module.fail_json(msg="VM check disks[%s]: new disk position %s is out of bounds!" % (position, disk_userdevice)) # For new disks we only track their position. @@ -1558,7 +1562,7 @@ class XenServerVM(XenServerObject): elif self.vm_params['customization_agent'] == "custom": vm_xenstore_data = self.vm_params['xenstore_data'] - if network_type and network_type != vm_xenstore_data.get('vm-data/networks/%s/type' % vm_vif_params['device'], ""): + if network_type and network_type != vm_xenstore_data.get('vm-data/networks/%s/type' % vm_vif_params['device'], "none"): network_changes.append('type') need_poweredoff = True @@ -1577,7 +1581,7 @@ class XenServerVM(XenServerObject): network_changes.append('gateway') need_poweredoff = True - if network_type6 and network_type6 != vm_xenstore_data.get('vm-data/networks/%s/type6' % vm_vif_params['device'], ""): + if network_type6 and network_type6 != vm_xenstore_data.get('vm-data/networks/%s/type6' % vm_vif_params['device'], "none"): network_changes.append('type6') need_poweredoff = True @@ -1616,6 +1620,9 @@ class XenServerVM(XenServerObject): need_poweredoff = True break + if not vif_devices_allowed: + self.module.fail_json(msg="VM check networks[%s]: maximum number of network interfaces reached!" % position) + # We need to place a new network interface right above the # highest placed existing interface to maintain relative # positions pairable with network interface specifications @@ -1721,7 +1728,7 @@ class XenServerVM(XenServerObject): # We found int value in string, let's typecast it. size = int(size) - if not size: + if not size or size < 0: raise ValueError except (TypeError, ValueError, NameError):