Ovirt: fixes #41592, #34559 (#41622)

* autostart cannot be used with command=define

autostart cannot be used with command=define. If it's used with state, it'll be silently ignored.
https://github.com/ansible/ansible/issues/41592

* guest name should not be used with xml

When both ```name``` and ```xml``` are specified, there might be a mismatch of domain names in
```name``` and inside ```xml```.

* Improved error handling and param deps for autostart-only tasks.
Standalone autostart tasks depend on ```name``` and need the domain to be present.

* Added handling of errors thrown by libvirt

* Updates to documentstion and examples

* Removed required flag from name. Added description on some option combinations.
Added a few examples.

* lint issues

* docs: note that name is optional

* Removing required from documentation of name parameter

* extra text to make clearer when name is required

* When defining a domain with xml, its name is taken directly from the xml definition.

This reverts commit 4ac14a622b.

lib/ansible/modules/cloud/misc/virt.py

@@ -31,6 +31,8 @@ options:
       - Note that there may be some lag for state requests like C(shutdown)
         since these refer only to VM states. After starting a guest, it may not
         be immediately accessible.
+        state and command are mutually exclusive except when command=list_vms. In
+        this case all VMs in specified state will be listed.
     choices: [ destroyed, paused, running, shutdown ]
   command:
     description:
@@ -69,20 +71,42 @@ EXAMPLES = '''
 # ansible host -m virt -a "name=alpha command=get_xml"
 # ansible host -m virt -a "name=alpha command=create uri=lxc:///"
 
----
+# defining and launching an LXC guest
-# a playbook example of defining and launching an LXC guest
+- name: define vm
-tasks:
+  virt:
-  - name: define vm
+    command: define
-    virt:
+    xml: "{{ lookup('template', 'container-template.xml.j2') }}"
-        name: foo
+    uri: 'lxc:///'
-        command: define
+- name: start vm
-        xml: "{{ lookup('template', 'container-template.xml.j2') }}"
+  virt:
-        uri: 'lxc:///'
+    name: foo
-  - name: start vm
+    state: running
-    virt:
+    uri: 'lxc:///'
-        name: foo
+
-        state: running
+# setting autostart on a qemu VM (default uri)
-        uri: 'lxc:///'
+- name: set autostart for a VM
+  virt:
+    name: foo
+    autostart: yes
+
+# Defining a VM and making is autostart with host. VM will be off after this task
+- name: define vm from xml and set autostart
+  virt:
+    command: define
+    xml: "{{ lookup('template', 'vm_template.xml.j2') }}"
+    autostart: yes
+
+# Listing VMs
+- name: list all VMs
+  virt:
+    command: list_vms
+  register: all_vms
+
+- name: list only running VMs
+  virt:
+    command: list_vms
+    state: running
+  register: running_vms
 '''
 
 RETURN = '''
@@ -107,11 +131,14 @@ import traceback
 
 try:
     import libvirt
+    from libvirt import libvirtError
 except ImportError:
     HAS_VIRT = False
 else:
     HAS_VIRT = True
 
+import re
+
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils._text import to_native
 
@@ -446,6 +473,17 @@ def core(module):
             res = {command: res}
         return VIRT_SUCCESS, res
 
+    if autostart is not None and command != 'define':
+        if not guest:
+            module.fail_json(msg="autostart requires 1 argument: name")
+        try:
+            v.get_vm(guest)
+        except VMNotFound:
+            module.fail_json(msg="domain %s not found" % guest)
+        res['changed'] = v.autostart(guest, autostart)
+        if not command and not state:
+            return VIRT_SUCCESS, res
+
     if state:
         if not guest:
             module.fail_json(msg="state change requires a guest specified")
@@ -474,25 +512,53 @@ def core(module):
 
         return VIRT_SUCCESS, res
 
-    if autostart is not None and v.autostart(guest, autostart):
-        res['changed'] = True
-
     if command:
         if command in VM_COMMANDS:
-            if not guest:
-                module.fail_json(msg="%s requires 1 argument: guest" % command)
             if command == 'define':
                 if not xml:
                     module.fail_json(msg="define requires xml argument")
+                if guest:
+                    # there might be a mismatch between quest 'name' in the module and in the xml
+                    module.warn("'xml' is given - ignoring 'name'")
+                found_name = re.search('<name>(.*)</name>', xml).groups()
+                if found_name:
+                    domain_name = found_name[0]
+                else:
+                    module.fail_json(msg="Could not find domain 'name' in xml")
+
+                # From libvirt docs (https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainDefineXML):
+                # -- A previous definition for this domain would be overridden if it already exists.
+                #
+                # In real world testing with libvirt versions 1.2.17-13, 2.0.0-10 and 3.9.0-14
+                # on qemu and lxc domains results in:
+                # operation failed: domain '<name>' already exists with <uuid>
+                #
+                # In case a domain would be indeed overwritten, we should protect idempotency:
                 try:
-                    v.get_vm(guest)
+                    existing_domain = v.get_vm(domain_name)
                 except VMNotFound:
-                    v.define(xml)
+                    existing_domain = None
-                    res = {'changed': True, 'created': guest}
+                try:
-                return VIRT_SUCCESS, res
+                    domain = v.define(xml)
-            res = getattr(v, command)(guest)
+                    if existing_domain:
-            if not isinstance(res, dict):
+                        # if we are here, then libvirt redefined existing domain as the doc promised
-                res = {command: res}
+                        if existing_domain.XMLDesc() != domain.XMLDesc():
+                            res = {'changed': True, 'change_reason': 'config changed'}
+                    else:
+                        res = {'changed': True, 'created': domain.name()}
+                except libvirtError as e:
+                    if e.get_error_code() != 9:  # 9 means 'domain already exists' error
+                        module.fail_json(msg='libvirtError: %s' % e.message)
+                if autostart is not None and v.autostart(domain_name, autostart):
+                    res = {'changed': True, 'change_reason': 'autostart'}
+
+            elif not guest:
+                module.fail_json(msg="%s requires 1 argument: guest" % command)
+            else:
+                res = getattr(v, command)(guest)
+                if not isinstance(res, dict):
+                    res = {command: res}
+
             return VIRT_SUCCESS, res
 
         elif hasattr(v, command):