XenServer: Minor bug fixes 2 (ansible#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

@@ -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)

lib/ansible/modules/cloud/xenserver/xenserver_guest.py

@@ -528,14 +528,14 @@ def deploy(self):
                 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 @@ def destroy(self):
         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 @@ def get_changes(self):
                                 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 @@ def get_changes(self):
                         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 @@ def get_changes(self):
                                     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 @@ def get_changes(self):
                                     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 @@ def get_normalized_disk_size(self, disk_params, msg_prefix=""):
                     # 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):