Skip to content

Commit

Permalink
Update MAC matching
Browse files Browse the repository at this point in the history 
Attempt to match nth interface in incoming XML with nth interface in
existing domain.

Update doc fragment to add warning to users that fuzzy matching may not
always result in the expected outcome.
  • Loading branch information
@mlow
mlow committed Oct 9, 2022
1 parent 36cbd1a commit 8bd508c
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 95 deletions.
11 changes: 8 additions & 3 deletions plugins/doc_fragments/virt.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,14 @@ class ModuleDocFragment(object):
- For each mutate_flag, we will modify the given XML in some way
- ADD_UUID will add an existing domain's UUID to the xml if it's missing
- ADD_MAC_ADDRESSES will look up interfaces in the existing domain with a
matching alias and copy the MAC address over
- ADD_MAC_ADDRESSES_FUZZY will try to match interfaces in the existing
domain based on matching type and source network/device
matching alias and copy the MAC address over. The matching interface
doesn't need to be of the same type or source network.
- ADD_MAC_ADDRESSES_FUZZY will try to match incoming interfaces with
interfaces of the existing domain sharing the same type and source
network/device. It may not always result in the expected outcome,
particularly if a domain has multiple interface attached to the same
host device and only some of those devices have <mac>s.
Use caution, do some testing for your particular use-case!
choices: [ ADD_UUID, ADD_MAC_ADDRESSES, ADD_MAC_ADDRESSES_FUZZY ]
type: list
elements: str
Expand Down
164 changes: 72 additions & 92 deletions plugins/modules/virt.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -478,53 +478,18 @@ def define(self, xml):
self.__get_conn()
return self.conn.define_from_xml(xml)


def _match_interface(match_interface, interfaces, fuzzy_match=False):
""" find match_interface in 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')

matched = None

match_alias = match_interface.find("./alias")
if match_alias is not None:
match_alias = match_alias.get('name')

_type = match_interface.get('type')
for interface in interfaces:
if interface.find('./mac') is None:
# this shouldn't happen since libvirt automatically assigns MACs
continue

if match_alias is not None:
# If the incoming interface has a alias,
interface_alias = interface.find('./alias')
if interface_alias is not None and interface_alias.get('name') == match_alias:
return interface

if not fuzzy_match or _type != interface.get('type'):
continue

if _type == 'network' and _source_network(interface) == _source_network(match_interface):
return interface
if _type == 'bridge' and _source_bridge(interface) == _source_bridge(match_interface):
return interface
if _type == 'direct' and _source_dev(interface) == _source_dev(match_interface):
return interface
# TODO: support additional types?

return None

# A dict of interface types (found in their `type` attribute) to the
# corresponding "source" attribute name of their <source> elements
# user networks don't have a <source> element
#
# We do not support fuzzy matching against any interface types
# not defined here
INTERFACE_SOURCE_ATTRS = {
'network': 'network',
'bridge': 'bridge',
'direct': 'dev',
'user': None,
}

def handle_define(module, v):
''' handle `command: define` '''
Expand Down Expand Up @@ -596,60 +561,75 @@ def handle_define(module, v):
module.fail_json(msg="attempting to re-define domain %s/%s with a different UUID: %s" % (
domain_name, existing_uuid, incoming_uuid
))

for flag in mutate_flags:
if flag == 'ADD_UUID' and incoming_uuid is None:
else:
if 'ADD_UUID' in mutate_flags and incoming_uuid is None:
# Users will often want to define their domains without an explicit
# UUID, instead giving them a unique name - so we support bringing
# over the UUID from the existing domain
etree.SubElement(incoming_xml, 'uuid').text = existing_uuid

elif flag == 'ADD_MAC_ADDRESSES' or flag == 'ADD_MAC_ADDRESSES_FUZZY':
# 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.
#
# With the ADD_MAC_ADDRESSES flag, we will attempt to copy the
# MAC addresses from the existing domain.

# if fuzzy_match is True, we will try to match an incoming
# interface to an existing based on whether the interface type and
# source device/network match.
# if fuzzy_match is False, we only match on aliases
fuzzy_match = flag == 'ADD_MAC_ADDRESSES_FUZZY'

# this set of nodes gets manipulated (elements removed), so be
# careful if using using `existing_xml` later
existing_devices = existing_xml.find('./devices')

# iterate user-defined <mac> elements
for mac in incoming_xml.findall('./devices/interface/mac'):
# remove from existing_devices any interfaces containing a
# MAC contained in incoming_xml, so we don't later match
# against one of these interfaces if a user provides MAC
# addresses for some but not all interfaces
for existing_interface in \
existing_devices.xpath('./interface[mac[@address="%s"]]' % mac.get('address')):
existing_devices.remove(existing_interface)

# iterate user-defined interfaces _without_ a mac element
for interface in incoming_xml.xpath('./devices/interface[not(mac)]'):
# incoming interface is missing MAC address, so we will add it
# from the existing domain if it has a matching interface
matched_interface = _match_interface(interface,
existing_devices.findall('./interface'),
fuzzy_match)

if matched_interface is not None:
# Remove matched_interface from possible candidates to prevent duplicate
# MAC address assignments (which is possible when fuzzy_match = True)
existing_devices.remove(matched_interface)
existing_devices = existing_xml.find('./devices')

if 'ADD_MAC_ADDRESSES' in mutate_flags:
for interface in incoming_xml.xpath('./devices/interface[not(mac) and alias]'):
search_alias = interface.find('alias').get('name')
xpath = "./interface[alias[@name='%s']]" % search_alias
try:
matched_interface = existing_devices.xpath(xpath)[0]
existing_devices.remove(matched_interface)
etree.SubElement(interface, 'mac', {
'address': matched_interface.find('mac').get('address')
})
except IndexError:
module.warn("Could not match interface %i of incoming XML by alias %s." % (
interface.getparent().index(interface) + 1, search_alias
))

if 'ADD_MAC_ADDRESSES_FUZZY' in mutate_flags:
# the counts of interfaces of a similar type/source
# key'd with tuple of (type, source)
similar_interface_counts = {}
def get_interface_count(_type, source=None):
key = (_type, source if _type != "user" else None)
if key not in similar_interface_counts:
similar_interface_counts[key] = 1
else:
similar_interface_counts[key] += 1
return similar_interface_counts[key]

# iterate user-defined interfaces
for interface in incoming_xml.xpath('./devices/interface'):
_type = interface.get('type')

if interface.find('mac') is not None and interface.find('alias') is not None:
continue

if _type not in INTERFACE_SOURCE_ATTRS:
module.warn("Skipping fuzzy MAC matching for interface %i of incoming XML: unsupported interface type '%s'" %(
interface.getparent().index(interface) + 1, _type
))
continue

source_attr = INTERFACE_SOURCE_ATTRS[_type]
source = interface.find('source').get(source_attr) if source_attr else None
similar_count = get_interface_count(_type, source)

if source:
xpath = "./interface[@type='%s' and source[@%s='%s']]" % (
_type, source_attr, source)
else:
xpath = "./interface[@type = '%s']" % source_attr

matching_interfaces = existing_devices.xpath(xpath)
try:
matched_interface = matching_interfaces[similar_count - 1]
etree.SubElement(interface, 'mac', {
'address': matched_interface.find('./mac').get('address'),
})
except IndexError:
module.warn("Could not fuzzy match interface %i of incoming XML." % (
interface.getparent().index(interface) + 1
))

try:
domain_xml = etree.tostring(incoming_xml).decode()
Expand Down

0 comments on commit 8bd508c

Please sign in to comment.