diff --git a/plugins/modules/virt.py b/plugins/modules/virt.py index d1b3596..e2dd35d 100644 --- a/plugins/modules/virt.py +++ b/plugins/modules/virt.py @@ -475,7 +475,43 @@ def define(self, xml): self.__get_conn() return self.conn.define_from_xml(xml) -def handle_define(module: AnsibleModule, v: Virt): + +def _lookup_interface_mac(match_interface, interfaces): + def _source_network(interface): + ''' return the libvirt network name for libvirt interfaces ''' + return interface.find('source').get('network') + + def _source_bridge(interface): + ''' return the bridge device for bridged interfaces ''' + return interface.find('source').get('bridge') + + def _source_dev(interface): + ''' return the source device for macvtap interfaces ''' + return interface.find('source').get('dev') + + def _get_mac(interface): + return interface.find('mac').get('address') + + matched = None + + _type = match_interface.get('type') + for interface in interfaces: + if _type != interface.get('type') or interface.find("mac") is None: + continue + + if _type == 'network' and _source_network(interface) == _source_network(match_interface): + return _get_mac(interface) + if _type == 'bridge' and _source_bridge(interface) == _source_bridge(match_interface): + return _get_mac(interface) + if _type == 'direct' and _source_dev(interface) == _source_dev(match_interface): + return _get_mac(interface) + # TODO: support additional types + + return None + + +def handle_define(module, v): + ''' handle `command: define` ''' xml = module.params.get('xml', None) guest = module.params.get('name', None) autostart = module.params.get('autostart', None) @@ -508,36 +544,106 @@ def handle_define(module: AnsibleModule, v: Virt): res = dict() # 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. + # -- A previous definition for this domain with the same UUID and name would + # be overridden if it already exists. + # + # If a domain is defined without a , libvirt will generate one for it. + # If an attempt is made to re-define the same xml (with the same and + # no ), libvirt will complain with the following error: # - # 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 '' already exists with # - # In case a domain would be indeed overwritten, we should protect idempotency: + # If a domain with a similiar but different is defined, + # libvirt complains with the same error. However, if a domain is defined + # with the same and as an existing domain, then libvirt will + # update the domain with the new definition (automatically handling + # addition/removal of devices. some changes may require a boot). + # + # Users will often want to define their domains without an explicit + # UUID, instead giving them a unique name. + # + # In similar fashion, libvirt will automatically generate MAC addresses for + # interfaces if they are defined without one. If a domain is re-defined with + # XML that is missing MAC addresses, libvirt will generate entirely new MAC + # addresses, which is probably not what most users want. + # + # Therefore, if and/or interface MAC addresess are missing from + # incoming xml, we will attempt to bring them over from the existing domain. + existing_xml = None + existing_xml_raw = None try: - existing_domain_xml = v.get_vm(domain_name).XMLDesc( - libvirt.VIR_DOMAIN_XML_INACTIVE - ) + existing_domain = v.get_vm(domain_name) except VMNotFound: - existing_domain_xml = None + existing_domain = None + + if existing_domain is not None: + # we are updating a domain's definition + incoming_uuid = incoming_xml.findtext('./uuid') + existing_uuid = existing_domain.UUIDString() + + if incoming_uuid is not None: + if incoming_uuid != existing_uuid: + # A user should not try defining a domain with the same name but + # different UUID + module.fail_json(msg="attempting to re-define domain %s/%s with a different UUID: %s" % ( + domain_name, existing_uuid, incoming_uuid + )) + else: + # UUID was missing in incoming xml, so add it + etree.SubElement(incoming_xml, 'uuid').text = existing_uuid + + existing_xml_raw = existing_domain.XMLDesc(libvirt.VIR_DOMAIN_XML_INACTIVE) + existing_xml = etree.fromstring(existing_xml_raw) + + # handle copying over MAC addresses from existing definition + existing_interfaces = existing_xml.findall('./devices/interface') + for interface in incoming_xml.findall('./devices/interface'): + mac = interface.find('mac') + if mac is None: + # incoming interface is missing MAC address, so we will add it + # from the existing domain if it has a matching interface + existing_mac = _lookup_interface_mac(interface, existing_interfaces) + if existing_mac: + etree.SubElement(interface, 'mac', { + 'address': existing_mac, + }) + try: - domain = v.define(xml) - if existing_domain_xml: - # if we are here, then libvirt redefined existing domain as the doc promised - new_domain_xml = domain.XMLDesc(libvirt.VIR_DOMAIN_XML_INACTIVE) - if existing_domain_xml != new_domain_xml: - res = {'changed': True, 'change_reason': 'config changed'} + domain_xml = etree.tostring(incoming_xml).decode() + + # TODO: support check mode + domain = v.define(domain_xml) + + if existing_xml is not None: + # In this case, we may have updated the definition or it might be the same. + # We compare the domain's previous xml with its new state and diff + # the changes. This allows users to fix their xml if it results in + # non-idempotent behaviour (e.g. libvirt mutates it each time) + new_xml = domain.XMLDesc(libvirt.VIR_DOMAIN_XML_INACTIVE) + if existing_xml_raw != new_xml: + res.update({ + 'changed': True, + 'change_reason': 'domain definition changed', + 'diff': { + 'before': existing_xml_raw, + 'after': new_xml + } + }) else: - res = {'changed': True, 'created': domain.name()} + # there was no existing XML, so this is a newly created domain + res.update({'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.get_error_message()) + module.fail_json(msg='libvirtError: %s' % e.get_error_message()) + except Exception as e: + module.fail_json(msg='an unknown error occured: %s' % e) + if autostart is not None and v.autostart(domain_name, autostart): - res = {'changed': True, 'change_reason': 'autostart'} + res.update({'changed': True, 'change_reason': 'autostart'}) return res + def core(module): state = module.params.get('state', None)