From 6e84fd53f5f04471de7dea1d10b00bebfdcba2e9 Mon Sep 17 00:00:00 2001
From: Cedric Bosdonnat <cbosdonnat@suse.com>
Date: Fri, 3 Jul 2020 14:43:44 +0200
Subject: [PATCH] Opensuse 3000 libvirt engine fixes (#248)

* Blacken libvirt-engines

* libvirt events engine: constant fixes

The libvirt constants for pool and nodedev events need tweaking since
they don't match the rule used to autogenerate them. Fixes these warnings
at the engine start.

   Skipping "pool/lifecycle" events: libvirt too old
   Skipping "pool/refresh" events: libvirt too old
   Skipping "nodedev/lifecycle" events: libvirt too old
   Skipping "nodedev/update" events: libvirt too old
---
 changelog/57746.fixed                     |   1 +
 salt/engines/libvirt_events.py            | 685 ++++++++++++----------
 tests/unit/engines/test_libvirt_events.py | 187 +++---
 3 files changed, 495 insertions(+), 378 deletions(-)
 create mode 100644 changelog/57746.fixed

diff --git a/changelog/57746.fixed b/changelog/57746.fixed
new file mode 100644
index 0000000000..5102bb04e9
--- /dev/null
+++ b/changelog/57746.fixed
@@ -0,0 +1 @@
+Fix the registration of libvirt pool and nodedev events
diff --git a/salt/engines/libvirt_events.py b/salt/engines/libvirt_events.py
index cdb5d1dfe8..96ba9efc91 100644
--- a/salt/engines/libvirt_events.py
+++ b/salt/engines/libvirt_events.py
@@ -1,6 +1,6 @@
 # -*- coding: utf-8 -*-
 
-'''
+"""
 An engine that listens for libvirt events and resends them to the salt event bus.
 
 The minimal configuration is the following and will listen to all events on the
@@ -35,7 +35,7 @@ CALLBACK_DEFS constant. If the filters list contains ``all``, all
 events will be relayed.
 
 Be aware that the list of events increases with libvirt versions, for example
-network events have been added in libvirt 1.2.1.
+network events have been added in libvirt 1.2.1 and storage events in 2.0.0.
 
 Running the engine on non-root
 ------------------------------
@@ -63,7 +63,7 @@ A polkit rule like the following one will allow `salt` user to connect to libvir
 :depends: libvirt 1.0.0+ python binding
 
 .. versionadded:: 2019.2.0
-'''
+"""
 
 from __future__ import absolute_import, unicode_literals, print_function
 import logging
@@ -73,6 +73,7 @@ import salt.utils.event
 
 # pylint: disable=no-name-in-module,import-error
 from salt.ext.six.moves.urllib.parse import urlparse
+
 # pylint: enable=no-name-in-module,import-error
 
 log = logging.getLogger(__name__)
@@ -85,112 +86,125 @@ except ImportError:
 
 
 def __virtual__():
-    '''
+    """
     Only load if libvirt python binding is present
-    '''
+    """
     if libvirt is None:
-        msg = 'libvirt module not found'
+        msg = "libvirt module not found"
     elif libvirt.getVersion() < 1000000:
-        msg = 'libvirt >= 1.0.0 required'
+        msg = "libvirt >= 1.0.0 required"
     else:
-        msg = ''
+        msg = ""
     return not bool(msg), msg
 
 
 REGISTER_FUNCTIONS = {
-    'domain': 'domainEventRegisterAny',
-    'network': 'networkEventRegisterAny',
-    'pool': 'storagePoolEventRegisterAny',
-    'nodedev': 'nodeDeviceEventRegisterAny',
-    'secret': 'secretEventRegisterAny'
+    "domain": "domainEventRegisterAny",
+    "network": "networkEventRegisterAny",
+    "pool": "storagePoolEventRegisterAny",
+    "nodedev": "nodeDeviceEventRegisterAny",
+    "secret": "secretEventRegisterAny",
 }
 
 # Handle either BLOCK_JOB or BLOCK_JOB_2, but prefer the latter
-if hasattr(libvirt, 'VIR_DOMAIN_EVENT_ID_BLOCK_JOB_2'):
-    BLOCK_JOB_ID = 'VIR_DOMAIN_EVENT_ID_BLOCK_JOB_2'
+if hasattr(libvirt, "VIR_DOMAIN_EVENT_ID_BLOCK_JOB_2"):
+    BLOCK_JOB_ID = "VIR_DOMAIN_EVENT_ID_BLOCK_JOB_2"
 else:
-    BLOCK_JOB_ID = 'VIR_DOMAIN_EVENT_ID_BLOCK_JOB'
+    BLOCK_JOB_ID = "VIR_DOMAIN_EVENT_ID_BLOCK_JOB"
 
 CALLBACK_DEFS = {
-    'domain': (('lifecycle', None),
-               ('reboot', None),
-               ('rtc_change', None),
-               ('watchdog', None),
-               ('graphics', None),
-               ('io_error', 'VIR_DOMAIN_EVENT_ID_IO_ERROR_REASON'),
-               ('control_error', None),
-               ('disk_change', None),
-               ('tray_change', None),
-               ('pmwakeup', None),
-               ('pmsuspend', None),
-               ('balloon_change', None),
-               ('pmsuspend_disk', None),
-               ('device_removed', None),
-               ('block_job', BLOCK_JOB_ID),
-               ('tunable', None),
-               ('agent_lifecycle', None),
-               ('device_added', None),
-               ('migration_iteration', None),
-               ('job_completed', None),
-               ('device_removal_failed', None),
-               ('metadata_change', None),
-               ('block_threshold', None)),
-    'network': (('lifecycle', None),),
-    'pool': (('lifecycle', None),
-             ('refresh', None)),
-    'nodedev': (('lifecycle', None),
-                ('update', None)),
-    'secret': (('lifecycle', None),
-               ('value_changed', None))
+    "domain": (
+        ("lifecycle", None),
+        ("reboot", None),
+        ("rtc_change", None),
+        ("watchdog", None),
+        ("graphics", None),
+        ("io_error", "VIR_DOMAIN_EVENT_ID_IO_ERROR_REASON"),
+        ("control_error", None),
+        ("disk_change", None),
+        ("tray_change", None),
+        ("pmwakeup", None),
+        ("pmsuspend", None),
+        ("balloon_change", None),
+        ("pmsuspend_disk", None),
+        ("device_removed", None),
+        ("block_job", BLOCK_JOB_ID),
+        ("tunable", None),
+        ("agent_lifecycle", None),
+        ("device_added", None),
+        ("migration_iteration", None),
+        ("job_completed", None),
+        ("device_removal_failed", None),
+        ("metadata_change", None),
+        ("block_threshold", None),
+    ),
+    "network": (("lifecycle", None),),
+    "pool": (
+        ("lifecycle", "VIR_STORAGE_POOL_EVENT_ID_LIFECYCLE"),
+        ("refresh", "VIR_STORAGE_POOL_EVENT_ID_REFRESH"),
+    ),
+    "nodedev": (
+        ("lifecycle", "VIR_NODE_DEVICE_EVENT_ID_LIFECYCLE"),
+        ("update", "VIR_NODE_DEVICE_EVENT_ID_UPDATE"),
+    ),
+    "secret": (("lifecycle", None), ("value_changed", None)),
 }
 
 
 def _compute_subprefix(attr):
-    '''
+    """
     Get the part before the first '_' or the end of attr including
     the potential '_'
-    '''
-    return ''.join((attr.split('_')[0], '_' if len(attr.split('_')) > 1 else ''))
+    """
+    return "".join((attr.split("_")[0], "_" if len(attr.split("_")) > 1 else ""))
 
 
 def _get_libvirt_enum_string(prefix, value):
-    '''
+    """
     Convert the libvirt enum integer value into a human readable string.
 
     :param prefix: start of the libvirt attribute to look for.
     :param value: integer to convert to string
-    '''
-    attributes = [attr[len(prefix):] for attr in libvirt.__dict__ if attr.startswith(prefix)]
+    """
+    attributes = [
+        attr[len(prefix) :] for attr in libvirt.__dict__ if attr.startswith(prefix)
+    ]
 
     # Filter out the values starting with a common base as they match another enum
     prefixes = [_compute_subprefix(p) for p in attributes]
     counts = {p: prefixes.count(p) for p in prefixes}
-    sub_prefixes = [p for p, count in counts.items() if count > 1 or (p.endswith('_') and p[:-1] in prefixes)]
-    filtered = [attr for attr in attributes if _compute_subprefix(attr) not in sub_prefixes]
+    sub_prefixes = [
+        p
+        for p, count in counts.items()
+        if count > 1 or (p.endswith("_") and p[:-1] in prefixes)
+    ]
+    filtered = [
+        attr for attr in attributes if _compute_subprefix(attr) not in sub_prefixes
+    ]
 
     for candidate in filtered:
-        if value == getattr(libvirt, ''.join((prefix, candidate))):
-            name = candidate.lower().replace('_', ' ')
+        if value == getattr(libvirt, "".join((prefix, candidate))):
+            name = candidate.lower().replace("_", " ")
             return name
-    return 'unknown'
+    return "unknown"
 
 
 def _get_domain_event_detail(event, detail):
-    '''
+    """
     Convert event and detail numeric values into a tuple of human readable strings
-    '''
-    event_name = _get_libvirt_enum_string('VIR_DOMAIN_EVENT_', event)
-    if event_name == 'unknown':
-        return event_name, 'unknown'
+    """
+    event_name = _get_libvirt_enum_string("VIR_DOMAIN_EVENT_", event)
+    if event_name == "unknown":
+        return event_name, "unknown"
 
-    prefix = 'VIR_DOMAIN_EVENT_{0}_'.format(event_name.upper())
+    prefix = "VIR_DOMAIN_EVENT_{0}_".format(event_name.upper())
     detail_name = _get_libvirt_enum_string(prefix, detail)
 
     return event_name, detail_name
 
 
 def _salt_send_event(opaque, conn, data):
-    '''
+    """
     Convenience function adding common data to the event and sending it
     on the salt event bus.
 
@@ -198,10 +212,10 @@ def _salt_send_event(opaque, conn, data):
                    This is a dict with 'prefix', 'object' and 'event' keys.
     :param conn: libvirt connection
     :param data: additional event data dict to send
-    '''
-    tag_prefix = opaque['prefix']
-    object_type = opaque['object']
-    event_type = opaque['event']
+    """
+    tag_prefix = opaque["prefix"]
+    object_type = opaque["object"]
+    event_type = opaque["event"]
 
     # Prepare the connection URI to fit in the tag
     # qemu+ssh://user@host:1234/system -> qemu+ssh/user@host:1234/system
@@ -209,30 +223,28 @@ def _salt_send_event(opaque, conn, data):
     uri_tag = [uri.scheme]
     if uri.netloc:
         uri_tag.append(uri.netloc)
-    path = uri.path.strip('/')
+    path = uri.path.strip("/")
     if path:
         uri_tag.append(path)
     uri_str = "/".join(uri_tag)
 
     # Append some common data
-    all_data = {
-        'uri': conn.getURI()
-    }
+    all_data = {"uri": conn.getURI()}
     all_data.update(data)
 
-    tag = '/'.join((tag_prefix, uri_str, object_type, event_type))
+    tag = "/".join((tag_prefix, uri_str, object_type, event_type))
 
     # Actually send the event in salt
-    if __opts__.get('__role') == 'master':
-        salt.utils.event.get_master_event(
-            __opts__,
-            __opts__['sock_dir']).fire_event(all_data, tag)
+    if __opts__.get("__role") == "master":
+        salt.utils.event.get_master_event(__opts__, __opts__["sock_dir"]).fire_event(
+            all_data, tag
+        )
     else:
-        __salt__['event.send'](tag, all_data)
+        __salt__["event.send"](tag, all_data)
 
 
 def _salt_send_domain_event(opaque, conn, domain, event, event_data):
-    '''
+    """
     Helper function send a salt event for a libvirt domain.
 
     :param opaque: the opaque data that is passed to the callback.
@@ -241,375 +253,428 @@ def _salt_send_domain_event(opaque, conn, domain, event, event_data):
     :param domain: name of the domain related to the event
     :param event: name of the event
     :param event_data: additional event data dict to send
-    '''
+    """
     data = {
-        'domain': {
-            'name': domain.name(),
-            'id': domain.ID(),
-            'uuid': domain.UUIDString()
+        "domain": {
+            "name": domain.name(),
+            "id": domain.ID(),
+            "uuid": domain.UUIDString(),
         },
-        'event': event
+        "event": event,
     }
     data.update(event_data)
     _salt_send_event(opaque, conn, data)
 
 
 def _domain_event_lifecycle_cb(conn, domain, event, detail, opaque):
-    '''
+    """
     Domain lifecycle events handler
-    '''
+    """
     event_str, detail_str = _get_domain_event_detail(event, detail)
 
-    _salt_send_domain_event(opaque, conn, domain, opaque['event'], {
-        'event':  event_str,
-        'detail': detail_str
-    })
+    _salt_send_domain_event(
+        opaque,
+        conn,
+        domain,
+        opaque["event"],
+        {"event": event_str, "detail": detail_str},
+    )
 
 
 def _domain_event_reboot_cb(conn, domain, opaque):
-    '''
+    """
     Domain reboot events handler
-    '''
-    _salt_send_domain_event(opaque, conn, domain, opaque['event'], {})
+    """
+    _salt_send_domain_event(opaque, conn, domain, opaque["event"], {})
 
 
 def _domain_event_rtc_change_cb(conn, domain, utcoffset, opaque):
-    '''
+    """
     Domain RTC change events handler
-    '''
-    _salt_send_domain_event(opaque, conn, domain, opaque['event'], {
-        'utcoffset': utcoffset
-    })
+    """
+    _salt_send_domain_event(
+        opaque, conn, domain, opaque["event"], {"utcoffset": utcoffset}
+    )
 
 
 def _domain_event_watchdog_cb(conn, domain, action, opaque):
-    '''
+    """
     Domain watchdog events handler
-    '''
-    _salt_send_domain_event(opaque, conn, domain, opaque['event'], {
-        'action': _get_libvirt_enum_string('VIR_DOMAIN_EVENT_WATCHDOG_', action)
-    })
+    """
+    _salt_send_domain_event(
+        opaque,
+        conn,
+        domain,
+        opaque["event"],
+        {"action": _get_libvirt_enum_string("VIR_DOMAIN_EVENT_WATCHDOG_", action)},
+    )
 
 
 def _domain_event_io_error_cb(conn, domain, srcpath, devalias, action, reason, opaque):
-    '''
+    """
     Domain I/O Error events handler
-    '''
-    _salt_send_domain_event(opaque, conn, domain, opaque['event'], {
-        'srcPath': srcpath,
-        'dev': devalias,
-        'action': _get_libvirt_enum_string('VIR_DOMAIN_EVENT_IO_ERROR_', action),
-        'reason': reason
-    })
+    """
+    _salt_send_domain_event(
+        opaque,
+        conn,
+        domain,
+        opaque["event"],
+        {
+            "srcPath": srcpath,
+            "dev": devalias,
+            "action": _get_libvirt_enum_string("VIR_DOMAIN_EVENT_IO_ERROR_", action),
+            "reason": reason,
+        },
+    )
 
 
-def _domain_event_graphics_cb(conn, domain, phase, local, remote, auth, subject, opaque):
-    '''
+def _domain_event_graphics_cb(
+    conn, domain, phase, local, remote, auth, subject, opaque
+):
+    """
     Domain graphics events handler
-    '''
-    prefix = 'VIR_DOMAIN_EVENT_GRAPHICS_'
+    """
+    prefix = "VIR_DOMAIN_EVENT_GRAPHICS_"
 
     def get_address(addr):
-        '''
+        """
         transform address structure into event data piece
-        '''
-        return {'family': _get_libvirt_enum_string('{0}_ADDRESS_'.format(prefix), addr['family']),
-                'node': addr['node'],
-                'service': addr['service']}
+        """
+        return {
+            "family": _get_libvirt_enum_string(
+                "{0}_ADDRESS_".format(prefix), addr["family"]
+            ),
+            "node": addr["node"],
+            "service": addr["service"],
+        }
 
-    _salt_send_domain_event(opaque, conn, domain, opaque['event'], {
-        'phase': _get_libvirt_enum_string(prefix, phase),
-        'local': get_address(local),
-        'remote': get_address(remote),
-        'authScheme': auth,
-        'subject': [{'type': item[0], 'name': item[1]} for item in subject]
-    })
+    _salt_send_domain_event(
+        opaque,
+        conn,
+        domain,
+        opaque["event"],
+        {
+            "phase": _get_libvirt_enum_string(prefix, phase),
+            "local": get_address(local),
+            "remote": get_address(remote),
+            "authScheme": auth,
+            "subject": [{"type": item[0], "name": item[1]} for item in subject],
+        },
+    )
 
 
 def _domain_event_control_error_cb(conn, domain, opaque):
-    '''
+    """
     Domain control error events handler
-    '''
-    _salt_send_domain_event(opaque, conn, domain, opaque['event'], {})
+    """
+    _salt_send_domain_event(opaque, conn, domain, opaque["event"], {})
 
 
 def _domain_event_disk_change_cb(conn, domain, old_src, new_src, dev, reason, opaque):
-    '''
+    """
     Domain disk change events handler
-    '''
-    _salt_send_domain_event(opaque, conn, domain, opaque['event'], {
-        'oldSrcPath': old_src,
-        'newSrcPath': new_src,
-        'dev': dev,
-        'reason': _get_libvirt_enum_string('VIR_DOMAIN_EVENT_DISK_', reason)
-    })
+    """
+    _salt_send_domain_event(
+        opaque,
+        conn,
+        domain,
+        opaque["event"],
+        {
+            "oldSrcPath": old_src,
+            "newSrcPath": new_src,
+            "dev": dev,
+            "reason": _get_libvirt_enum_string("VIR_DOMAIN_EVENT_DISK_", reason),
+        },
+    )
 
 
 def _domain_event_tray_change_cb(conn, domain, dev, reason, opaque):
-    '''
+    """
     Domain tray change events handler
-    '''
-    _salt_send_domain_event(opaque, conn, domain, opaque['event'], {
-        'dev': dev,
-        'reason': _get_libvirt_enum_string('VIR_DOMAIN_EVENT_TRAY_CHANGE_', reason)
-    })
+    """
+    _salt_send_domain_event(
+        opaque,
+        conn,
+        domain,
+        opaque["event"],
+        {
+            "dev": dev,
+            "reason": _get_libvirt_enum_string("VIR_DOMAIN_EVENT_TRAY_CHANGE_", reason),
+        },
+    )
 
 
 def _domain_event_pmwakeup_cb(conn, domain, reason, opaque):
-    '''
+    """
     Domain wakeup events handler
-    '''
-    _salt_send_domain_event(opaque, conn, domain, opaque['event'], {
-        'reason': 'unknown'  # currently unused
-    })
+    """
+    _salt_send_domain_event(
+        opaque, conn, domain, opaque["event"], {"reason": "unknown"}  # currently unused
+    )
 
 
 def _domain_event_pmsuspend_cb(conn, domain, reason, opaque):
-    '''
+    """
     Domain suspend events handler
-    '''
-    _salt_send_domain_event(opaque, conn, domain, opaque['event'], {
-        'reason': 'unknown'  # currently unused
-    })
+    """
+    _salt_send_domain_event(
+        opaque, conn, domain, opaque["event"], {"reason": "unknown"}  # currently unused
+    )
 
 
 def _domain_event_balloon_change_cb(conn, domain, actual, opaque):
-    '''
+    """
     Domain balloon change events handler
-    '''
-    _salt_send_domain_event(opaque, conn, domain, opaque['event'], {
-        'actual': actual
-    })
+    """
+    _salt_send_domain_event(opaque, conn, domain, opaque["event"], {"actual": actual})
 
 
 def _domain_event_pmsuspend_disk_cb(conn, domain, reason, opaque):
-    '''
+    """
     Domain disk suspend events handler
-    '''
-    _salt_send_domain_event(opaque, conn, domain, opaque['event'], {
-        'reason': 'unknown'  # currently unused
-    })
+    """
+    _salt_send_domain_event(
+        opaque, conn, domain, opaque["event"], {"reason": "unknown"}  # currently unused
+    )
 
 
 def _domain_event_block_job_cb(conn, domain, disk, job_type, status, opaque):
-    '''
+    """
     Domain block job events handler
-    '''
-    _salt_send_domain_event(opaque, conn, domain, opaque['event'], {
-        'disk': disk,
-        'type': _get_libvirt_enum_string('VIR_DOMAIN_BLOCK_JOB_TYPE_', job_type),
-        'status': _get_libvirt_enum_string('VIR_DOMAIN_BLOCK_JOB_', status)
-    })
+    """
+    _salt_send_domain_event(
+        opaque,
+        conn,
+        domain,
+        opaque["event"],
+        {
+            "disk": disk,
+            "type": _get_libvirt_enum_string("VIR_DOMAIN_BLOCK_JOB_TYPE_", job_type),
+            "status": _get_libvirt_enum_string("VIR_DOMAIN_BLOCK_JOB_", status),
+        },
+    )
 
 
 def _domain_event_device_removed_cb(conn, domain, dev, opaque):
-    '''
+    """
     Domain device removal events handler
-    '''
-    _salt_send_domain_event(opaque, conn, domain, opaque['event'], {
-        'dev': dev
-    })
+    """
+    _salt_send_domain_event(opaque, conn, domain, opaque["event"], {"dev": dev})
 
 
 def _domain_event_tunable_cb(conn, domain, params, opaque):
-    '''
+    """
     Domain tunable events handler
-    '''
-    _salt_send_domain_event(opaque, conn, domain, opaque['event'], {
-        'params': params
-    })
+    """
+    _salt_send_domain_event(opaque, conn, domain, opaque["event"], {"params": params})
 
 
 # pylint: disable=invalid-name
 def _domain_event_agent_lifecycle_cb(conn, domain, state, reason, opaque):
-    '''
+    """
     Domain agent lifecycle events handler
-    '''
-    _salt_send_domain_event(opaque, conn, domain, opaque['event'], {
-        'state': _get_libvirt_enum_string('VIR_CONNECT_DOMAIN_EVENT_AGENT_LIFECYCLE_STATE_', state),
-        'reason': _get_libvirt_enum_string('VIR_CONNECT_DOMAIN_EVENT_AGENT_LIFECYCLE_REASON_', reason)
-    })
+    """
+    _salt_send_domain_event(
+        opaque,
+        conn,
+        domain,
+        opaque["event"],
+        {
+            "state": _get_libvirt_enum_string(
+                "VIR_CONNECT_DOMAIN_EVENT_AGENT_LIFECYCLE_STATE_", state
+            ),
+            "reason": _get_libvirt_enum_string(
+                "VIR_CONNECT_DOMAIN_EVENT_AGENT_LIFECYCLE_REASON_", reason
+            ),
+        },
+    )
 
 
 def _domain_event_device_added_cb(conn, domain, dev, opaque):
-    '''
+    """
     Domain device addition events handler
-    '''
-    _salt_send_domain_event(opaque, conn, domain, opaque['event'], {
-        'dev': dev
-    })
+    """
+    _salt_send_domain_event(opaque, conn, domain, opaque["event"], {"dev": dev})
 
 
 # pylint: disable=invalid-name
 def _domain_event_migration_iteration_cb(conn, domain, iteration, opaque):
-    '''
+    """
     Domain migration iteration events handler
-    '''
-    _salt_send_domain_event(opaque, conn, domain, opaque['event'], {
-        'iteration': iteration
-    })
+    """
+    _salt_send_domain_event(
+        opaque, conn, domain, opaque["event"], {"iteration": iteration}
+    )
 
 
 def _domain_event_job_completed_cb(conn, domain, params, opaque):
-    '''
+    """
     Domain job completion events handler
-    '''
-    _salt_send_domain_event(opaque, conn, domain, opaque['event'], {
-        'params': params
-    })
+    """
+    _salt_send_domain_event(opaque, conn, domain, opaque["event"], {"params": params})
 
 
 def _domain_event_device_removal_failed_cb(conn, domain, dev, opaque):
-    '''
+    """
     Domain device removal failure events handler
-    '''
-    _salt_send_domain_event(opaque, conn, domain, opaque['event'], {
-        'dev': dev
-    })
+    """
+    _salt_send_domain_event(opaque, conn, domain, opaque["event"], {"dev": dev})
 
 
 def _domain_event_metadata_change_cb(conn, domain, mtype, nsuri, opaque):
-    '''
+    """
     Domain metadata change events handler
-    '''
-    _salt_send_domain_event(opaque, conn, domain, opaque['event'], {
-        'type': _get_libvirt_enum_string('VIR_DOMAIN_METADATA_', mtype),
-        'nsuri': nsuri
-    })
+    """
+    _salt_send_domain_event(
+        opaque,
+        conn,
+        domain,
+        opaque["event"],
+        {
+            "type": _get_libvirt_enum_string("VIR_DOMAIN_METADATA_", mtype),
+            "nsuri": nsuri,
+        },
+    )
 
 
-def _domain_event_block_threshold_cb(conn, domain, dev, path, threshold, excess, opaque):
-    '''
+def _domain_event_block_threshold_cb(
+    conn, domain, dev, path, threshold, excess, opaque
+):
+    """
     Domain block threshold events handler
-    '''
-    _salt_send_domain_event(opaque, conn, domain, opaque['event'], {
-        'dev': dev,
-        'path': path,
-        'threshold': threshold,
-        'excess': excess
-    })
+    """
+    _salt_send_domain_event(
+        opaque,
+        conn,
+        domain,
+        opaque["event"],
+        {"dev": dev, "path": path, "threshold": threshold, "excess": excess},
+    )
 
 
 def _network_event_lifecycle_cb(conn, net, event, detail, opaque):
-    '''
+    """
     Network lifecycle events handler
-    '''
-
-    _salt_send_event(opaque, conn, {
-        'network': {
-            'name': net.name(),
-            'uuid': net.UUIDString()
+    """
+
+    _salt_send_event(
+        opaque,
+        conn,
+        {
+            "network": {"name": net.name(), "uuid": net.UUIDString()},
+            "event": _get_libvirt_enum_string("VIR_NETWORK_EVENT_", event),
+            "detail": "unknown",  # currently unused
         },
-        'event': _get_libvirt_enum_string('VIR_NETWORK_EVENT_', event),
-        'detail': 'unknown'  # currently unused
-    })
+    )
 
 
 def _pool_event_lifecycle_cb(conn, pool, event, detail, opaque):
-    '''
+    """
     Storage pool lifecycle events handler
-    '''
-    _salt_send_event(opaque, conn, {
-        'pool': {
-            'name': pool.name(),
-            'uuid': pool.UUIDString()
+    """
+    _salt_send_event(
+        opaque,
+        conn,
+        {
+            "pool": {"name": pool.name(), "uuid": pool.UUIDString()},
+            "event": _get_libvirt_enum_string("VIR_STORAGE_POOL_EVENT_", event),
+            "detail": "unknown",  # currently unused
         },
-        'event': _get_libvirt_enum_string('VIR_STORAGE_POOL_EVENT_', event),
-        'detail': 'unknown'  # currently unused
-    })
+    )
 
 
 def _pool_event_refresh_cb(conn, pool, opaque):
-    '''
+    """
     Storage pool refresh events handler
-    '''
-    _salt_send_event(opaque, conn, {
-        'pool': {
-            'name': pool.name(),
-            'uuid': pool.UUIDString()
+    """
+    _salt_send_event(
+        opaque,
+        conn,
+        {
+            "pool": {"name": pool.name(), "uuid": pool.UUIDString()},
+            "event": opaque["event"],
         },
-        'event': opaque['event']
-    })
+    )
 
 
 def _nodedev_event_lifecycle_cb(conn, dev, event, detail, opaque):
-    '''
+    """
     Node device lifecycle events handler
-    '''
-    _salt_send_event(opaque, conn, {
-        'nodedev': {
-            'name': dev.name()
+    """
+    _salt_send_event(
+        opaque,
+        conn,
+        {
+            "nodedev": {"name": dev.name()},
+            "event": _get_libvirt_enum_string("VIR_NODE_DEVICE_EVENT_", event),
+            "detail": "unknown",  # currently unused
         },
-        'event': _get_libvirt_enum_string('VIR_NODE_DEVICE_EVENT_', event),
-        'detail': 'unknown'  # currently unused
-    })
+    )
 
 
 def _nodedev_event_update_cb(conn, dev, opaque):
-    '''
+    """
     Node device update events handler
-    '''
-    _salt_send_event(opaque, conn, {
-        'nodedev': {
-            'name': dev.name()
-        },
-        'event': opaque['event']
-    })
+    """
+    _salt_send_event(
+        opaque, conn, {"nodedev": {"name": dev.name()}, "event": opaque["event"]}
+    )
 
 
 def _secret_event_lifecycle_cb(conn, secret, event, detail, opaque):
-    '''
+    """
     Secret lifecycle events handler
-    '''
-    _salt_send_event(opaque, conn, {
-        'secret': {
-            'uuid': secret.UUIDString()
+    """
+    _salt_send_event(
+        opaque,
+        conn,
+        {
+            "secret": {"uuid": secret.UUIDString()},
+            "event": _get_libvirt_enum_string("VIR_SECRET_EVENT_", event),
+            "detail": "unknown",  # currently unused
         },
-        'event': _get_libvirt_enum_string('VIR_SECRET_EVENT_', event),
-        'detail': 'unknown'  # currently unused
-    })
+    )
 
 
 def _secret_event_value_changed_cb(conn, secret, opaque):
-    '''
+    """
     Secret value change events handler
-    '''
-    _salt_send_event(opaque, conn, {
-        'secret': {
-            'uuid': secret.UUIDString()
-        },
-        'event': opaque['event']
-    })
+    """
+    _salt_send_event(
+        opaque,
+        conn,
+        {"secret": {"uuid": secret.UUIDString()}, "event": opaque["event"]},
+    )
 
 
 def _cleanup(cnx):
-    '''
+    """
     Close the libvirt connection
 
     :param cnx: libvirt connection
-    '''
-    log.debug('Closing libvirt connection: %s', cnx.getURI())
+    """
+    log.debug("Closing libvirt connection: %s", cnx.getURI())
     cnx.close()
 
 
 def _callbacks_cleanup(cnx, callback_ids):
-    '''
+    """
     Unregister all the registered callbacks
 
     :param cnx: libvirt connection
     :param callback_ids: dictionary mapping a libvirt object type to an ID list
                          of callbacks to deregister
-    '''
+    """
     for obj, ids in callback_ids.items():
         register_name = REGISTER_FUNCTIONS[obj]
-        deregister_name = register_name.replace('Reg', 'Dereg')
+        deregister_name = register_name.replace("Reg", "Dereg")
         deregister = getattr(cnx, deregister_name)
         for callback_id in ids:
             deregister(callback_id)
 
 
 def _register_callback(cnx, tag_prefix, obj, event, real_id):
-    '''
+    """
     Helper function registering a callback
 
     :param cnx: libvirt connection
@@ -620,10 +685,10 @@ def _register_callback(cnx, tag_prefix, obj, event, real_id):
     :param real_id: the libvirt name of an alternative event id to use or None
 
     :rtype integer value needed to deregister the callback
-    '''
+    """
     libvirt_name = real_id
     if real_id is None:
-        libvirt_name = 'VIR_{0}_EVENT_ID_{1}'.format(obj, event).upper()
+        libvirt_name = "VIR_{0}_EVENT_ID_{1}".format(obj, event).upper()
 
     if not hasattr(libvirt, libvirt_name):
         log.warning('Skipping "%s/%s" events: libvirt too old', obj, event)
@@ -633,34 +698,34 @@ def _register_callback(cnx, tag_prefix, obj, event, real_id):
     callback_name = "_{0}_event_{1}_cb".format(obj, event)
     callback = globals().get(callback_name, None)
     if callback is None:
-        log.error('Missing function %s in engine', callback_name)
+        log.error("Missing function %s in engine", callback_name)
         return None
 
     register = getattr(cnx, REGISTER_FUNCTIONS[obj])
-    return register(None, libvirt_id, callback,
-                    {'prefix': tag_prefix,
-                     'object': obj,
-                     'event': event})
+    return register(
+        None,
+        libvirt_id,
+        callback,
+        {"prefix": tag_prefix, "object": obj, "event": event},
+    )
 
 
 def _append_callback_id(ids, obj, callback_id):
-    '''
+    """
     Helper function adding a callback ID to the IDs dict.
     The callback ids dict maps an object to event callback ids.
 
     :param ids: dict of callback IDs to update
     :param obj: one of the keys of REGISTER_FUNCTIONS
     :param callback_id: the result of _register_callback
-    '''
+    """
     if obj not in ids:
         ids[obj] = []
     ids[obj].append(callback_id)
 
 
-def start(uri=None,
-          tag_prefix='salt/engines/libvirt_events',
-          filters=None):
-    '''
+def start(uri=None, tag_prefix="salt/engines/libvirt_events", filters=None):
+    """
     Listen to libvirt events and forward them to salt.
 
     :param uri: libvirt URI to listen on.
@@ -668,14 +733,14 @@ def start(uri=None,
     :param tag_prefix: the begining of the salt event tag to use.
                        Defaults to 'salt/engines/libvirt_events'
     :param filters: the list of event of listen on. Defaults to 'all'
-    '''
+    """
     if filters is None:
-        filters = ['all']
+        filters = ["all"]
     try:
         libvirt.virEventRegisterDefaultImpl()
 
         cnx = libvirt.openReadOnly(uri)
-        log.debug('Opened libvirt uri: %s', cnx.getURI())
+        log.debug("Opened libvirt uri: %s", cnx.getURI())
 
         callback_ids = {}
         all_filters = "all" in filters
@@ -683,17 +748,19 @@ def start(uri=None,
         for obj, event_defs in CALLBACK_DEFS.items():
             for event, real_id in event_defs:
                 event_filter = "/".join((obj, event))
-                if event_filter not in filters and obj not in filters and not all_filters:
+                if (
+                    event_filter not in filters
+                    and obj not in filters
+                    and not all_filters
+                ):
                     continue
-                registered_id = _register_callback(cnx, tag_prefix,
-                                                   obj, event, real_id)
+                registered_id = _register_callback(cnx, tag_prefix, obj, event, real_id)
                 if registered_id:
                     _append_callback_id(callback_ids, obj, registered_id)
 
         exit_loop = False
         while not exit_loop:
             exit_loop = libvirt.virEventRunDefaultImpl() < 0
-            log.debug('=== in the loop exit_loop %s ===', exit_loop)
 
     except Exception as err:  # pylint: disable=broad-except
         log.exception(err)
diff --git a/tests/unit/engines/test_libvirt_events.py b/tests/unit/engines/test_libvirt_events.py
index d9143a320b..5f1488e422 100644
--- a/tests/unit/engines/test_libvirt_events.py
+++ b/tests/unit/engines/test_libvirt_events.py
@@ -1,16 +1,14 @@
 # -*- coding: utf-8 -*-
-'''
+"""
 unit tests for the libvirt_events engine
-'''
+"""
 # Import Python libs
 from __future__ import absolute_import, print_function, unicode_literals
 
 # Import Salt Testing Libs
 from tests.support.mixins import LoaderModuleMockMixin
 from tests.support.unit import TestCase
-from tests.support.mock import (
-    MagicMock,
-    patch)
+from tests.support.mock import MagicMock, patch
 
 # Import Salt Libs
 import salt.engines.libvirt_events as libvirt_events
@@ -20,68 +18,78 @@ import salt.engines.libvirt_events as libvirt_events
 
 
 class EngineLibvirtEventTestCase(TestCase, LoaderModuleMockMixin):
-    '''
+    """
     Test cases for salt.engine.libvirt_events
-    '''
+    """
 
     def setup_loader_modules(self):
-        patcher = patch('salt.engines.libvirt_events.libvirt')
+        patcher = patch("salt.engines.libvirt_events.libvirt")
         self.mock_libvirt = patcher.start()
         self.mock_libvirt.getVersion.return_value = 2000000
-        self.mock_libvirt.virEventRunDefaultImpl.return_value = -1  # Don't loop for ever
+        self.mock_libvirt.virEventRunDefaultImpl.return_value = (
+            -1
+        )  # Don't loop for ever
         self.mock_libvirt.VIR_DOMAIN_EVENT_ID_LIFECYCLE = 0
         self.mock_libvirt.VIR_DOMAIN_EVENT_ID_REBOOT = 1
+        self.mock_libvirt.VIR_STORAGE_POOL_EVENT_ID_LIFECYCLE = 0
+        self.mock_libvirt.VIR_STORAGE_POOL_EVENT_ID_REFRESH = 1
+        self.mock_libvirt.VIR_NODE_DEVICE_EVENT_ID_LIFECYCLE = 0
+        self.mock_libvirt.VIR_NODE_DEVICE_EVENT_ID_UPDATE = 1
         self.addCleanup(patcher.stop)
-        self.addCleanup(delattr, self, 'mock_libvirt')
+        self.addCleanup(delattr, self, "mock_libvirt")
         return {libvirt_events: {}}
 
-    @patch('salt.engines.libvirt_events.libvirt',
-           VIR_PREFIX_NONE=0,
-           VIR_PREFIX_ONE=1,
-           VIR_PREFIX_TWO=2,
-           VIR_PREFIX_SUB_FOO=0,
-           VIR_PREFIX_SUB_BAR=1,
-           VIR_PREFIX_SUB_FOOBAR=2)
+    @patch(
+        "salt.engines.libvirt_events.libvirt",
+        VIR_PREFIX_NONE=0,
+        VIR_PREFIX_ONE=1,
+        VIR_PREFIX_TWO=2,
+        VIR_PREFIX_SUB_FOO=0,
+        VIR_PREFIX_SUB_BAR=1,
+        VIR_PREFIX_SUB_FOOBAR=2,
+    )
     def test_get_libvirt_enum_string_subprefix(self, libvirt_mock):
-        '''
+        """
         Make sure the libvirt enum value to string works reliably with
         elements with a sub prefix, eg VIR_PREFIX_SUB_* in this case.
-        '''
+        """
         # Test case with a sub prefix
 
-        assert libvirt_events._get_libvirt_enum_string('VIR_PREFIX_', 2) == 'two'
+        assert libvirt_events._get_libvirt_enum_string("VIR_PREFIX_", 2) == "two"
 
-    @patch('salt.engines.libvirt_events.libvirt',
-           VIR_PREFIX_FOO=0,
-           VIR_PREFIX_BAR_FOO=1)
+    @patch(
+        "salt.engines.libvirt_events.libvirt", VIR_PREFIX_FOO=0, VIR_PREFIX_BAR_FOO=1
+    )
     def test_get_libvirt_enum_string_underscores(self, libvirt_mock):
-        '''
+        """
         Make sure the libvirt enum value to string works reliably and items
         with an underscore aren't confused with sub prefixes.
-        '''
-        assert libvirt_events._get_libvirt_enum_string('VIR_PREFIX_', 1) == 'bar foo'
-
-    @patch('salt.engines.libvirt_events.libvirt',
-           VIR_DOMAIN_EVENT_CRASHED_PANICKED=0,
-           VIR_DOMAIN_EVENT_DEFINED=0,
-           VIR_DOMAIN_EVENT_UNDEFINED=1,
-           VIR_DOMAIN_EVENT_CRASHED=2,
-           VIR_DOMAIN_EVENT_DEFINED_ADDED=0,
-           VIR_DOMAIN_EVENT_DEFINED_UPDATED=1)
+        """
+        assert libvirt_events._get_libvirt_enum_string("VIR_PREFIX_", 1) == "bar foo"
+
+    @patch(
+        "salt.engines.libvirt_events.libvirt",
+        VIR_DOMAIN_EVENT_CRASHED_PANICKED=0,
+        VIR_DOMAIN_EVENT_DEFINED=0,
+        VIR_DOMAIN_EVENT_UNDEFINED=1,
+        VIR_DOMAIN_EVENT_CRASHED=2,
+        VIR_DOMAIN_EVENT_DEFINED_ADDED=0,
+        VIR_DOMAIN_EVENT_DEFINED_UPDATED=1,
+    )
     def test_get_domain_event_detail(self, mock_libvirt):
-        '''
+        """
         Test get_domain_event_detail function
-        '''
-        assert libvirt_events._get_domain_event_detail(1, 2) == ('undefined', 'unknown')
-        assert libvirt_events._get_domain_event_detail(0, 1) == ('defined', 'updated')
-        assert libvirt_events._get_domain_event_detail(4, 2) == ('unknown', 'unknown')
+        """
+        assert libvirt_events._get_domain_event_detail(1, 2) == ("undefined", "unknown")
+        assert libvirt_events._get_domain_event_detail(0, 1) == ("defined", "updated")
+        assert libvirt_events._get_domain_event_detail(4, 2) == ("unknown", "unknown")
 
-    @patch('salt.engines.libvirt_events.libvirt', VIR_NETWORK_EVENT_ID_LIFECYCLE=1000)
+    @patch("salt.engines.libvirt_events.libvirt", VIR_NETWORK_EVENT_ID_LIFECYCLE=1000)
     def test_event_register(self, mock_libvirt):
-        '''
+        """
         Test that the libvirt_events engine actually registers events catch them and cleans
         before leaving the place.
-        '''
+        """
         mock_cnx = MagicMock()
         mock_libvirt.openReadOnly.return_value = mock_cnx
 
@@ -90,71 +98,112 @@ class EngineLibvirtEventTestCase(TestCase, LoaderModuleMockMixin):
 
         mock_cnx.networkEventRegisterAny.return_value = 10000
 
-        libvirt_events.start('test:///', 'test/prefix')
+        libvirt_events.start("test:///", "test/prefix")
 
         # Check that the connection has been opened
-        mock_libvirt.openReadOnly.assert_called_once_with('test:///')
+        mock_libvirt.openReadOnly.assert_called_once_with("test:///")
 
         # Check that the connection has been closed
         mock_cnx.close.assert_called_once()
 
         # Check events registration and deregistration
         mock_cnx.domainEventRegisterAny.assert_any_call(
-                None, mock_libvirt.VIR_DOMAIN_EVENT_ID_LIFECYCLE,
-                libvirt_events._domain_event_lifecycle_cb,
-                {'prefix': 'test/prefix', 'object': 'domain', 'event': 'lifecycle'})
+            None,
+            mock_libvirt.VIR_DOMAIN_EVENT_ID_LIFECYCLE,
+            libvirt_events._domain_event_lifecycle_cb,
+            {"prefix": "test/prefix", "object": "domain", "event": "lifecycle"},
+        )
         mock_cnx.networkEventRegisterAny.assert_any_call(
-                None, mock_libvirt.VIR_NETWORK_EVENT_ID_LIFECYCLE,
-                libvirt_events._network_event_lifecycle_cb,
-                {'prefix': 'test/prefix', 'object': 'network', 'event': 'lifecycle'})
+            None,
+            mock_libvirt.VIR_NETWORK_EVENT_ID_LIFECYCLE,
+            libvirt_events._network_event_lifecycle_cb,
+            {"prefix": "test/prefix", "object": "network", "event": "lifecycle"},
+        )
+        mock_cnx.storagePoolEventRegisterAny.assert_any_call(
+            None,
+            mock_libvirt.VIR_STORAGE_POOL_EVENT_ID_LIFECYCLE,
+            libvirt_events._pool_event_lifecycle_cb,
+            {"prefix": "test/prefix", "object": "pool", "event": "lifecycle"},
+        )
+        mock_cnx.storagePoolEventRegisterAny.assert_any_call(
+            None,
+            mock_libvirt.VIR_STORAGE_POOL_EVENT_ID_REFRESH,
+            libvirt_events._pool_event_refresh_cb,
+            {"prefix": "test/prefix", "object": "pool", "event": "refresh"},
+        )
+        mock_cnx.nodeDeviceEventRegisterAny.assert_any_call(
+            None,
+            mock_libvirt.VIR_NODE_DEVICE_EVENT_ID_LIFECYCLE,
+            libvirt_events._nodedev_event_lifecycle_cb,
+            {"prefix": "test/prefix", "object": "nodedev", "event": "lifecycle"},
+        )
+        mock_cnx.nodeDeviceEventRegisterAny.assert_any_call(
+            None,
+            mock_libvirt.VIR_NODE_DEVICE_EVENT_ID_UPDATE,
+            libvirt_events._nodedev_event_update_cb,
+            {"prefix": "test/prefix", "object": "nodedev", "event": "update"},
+        )
 
         # Check that the deregister events are called with the result of register
         mock_cnx.networkEventDeregisterAny.assert_called_with(
-                mock_cnx.networkEventRegisterAny.return_value)
+            mock_cnx.networkEventRegisterAny.return_value
+        )
 
         # Check that the default 'all' filter actually worked
-        counts = {obj: len(callback_def) for obj, callback_def in libvirt_events.CALLBACK_DEFS.items()}
+        counts = {
+            obj: len(callback_def)
+            for obj, callback_def in libvirt_events.CALLBACK_DEFS.items()
+        }
         for obj, count in counts.items():
             register = libvirt_events.REGISTER_FUNCTIONS[obj]
             assert getattr(mock_cnx, register).call_count == count
 
     def test_event_skipped(self):
-        '''
+        """
         Test that events are skipped if their ID isn't defined in the libvirt
         module (older libvirt)
-        '''
-        self.mock_libvirt.mock_add_spec([
-                'openReadOnly',
-                'virEventRegisterDefaultImpl',
-                'virEventRunDefaultImpl',
-                'VIR_DOMAIN_EVENT_ID_LIFECYCLE'], spec_set=True)
-
-        libvirt_events.start('test:///', 'test/prefix')
+        """
+        self.mock_libvirt.mock_add_spec(
+            [
+                "openReadOnly",
+                "virEventRegisterDefaultImpl",
+                "virEventRunDefaultImpl",
+                "VIR_DOMAIN_EVENT_ID_LIFECYCLE",
+            ],
+            spec_set=True,
+        )
+
+        libvirt_events.start("test:///", "test/prefix")
 
         # Check events registration and deregistration
         mock_cnx = self.mock_libvirt.openReadOnly.return_value
 
         mock_cnx.domainEventRegisterAny.assert_any_call(
-                None, self.mock_libvirt.VIR_DOMAIN_EVENT_ID_LIFECYCLE,
-                libvirt_events._domain_event_lifecycle_cb,
-                {'prefix': 'test/prefix', 'object': 'domain', 'event': 'lifecycle'})
+            None,
+            self.mock_libvirt.VIR_DOMAIN_EVENT_ID_LIFECYCLE,
+            libvirt_events._domain_event_lifecycle_cb,
+            {"prefix": "test/prefix", "object": "domain", "event": "lifecycle"},
+        )
 
         # Network events should have been skipped
         mock_cnx.networkEventRegisterAny.assert_not_called()
 
     def test_event_filtered(self):
-        '''
+        """
         Test that events are skipped if their ID isn't defined in the libvirt
         module (older libvirt)
-        '''
-        libvirt_events.start('test', 'test/prefix', 'domain/lifecycle')
+        """
+        libvirt_events.start("test", "test/prefix", "domain/lifecycle")
 
         # Check events registration and deregistration
         mock_cnx = self.mock_libvirt.openReadOnly.return_value
 
         mock_cnx.domainEventRegisterAny.assert_any_call(
-                None, 0, libvirt_events._domain_event_lifecycle_cb,
-                {'prefix': 'test/prefix', 'object': 'domain', 'event': 'lifecycle'})
+            None,
+            0,
+            libvirt_events._domain_event_lifecycle_cb,
+            {"prefix": "test/prefix", "object": "domain", "event": "lifecycle"},
+        )
 
         # Network events should have been filtered out
         mock_cnx.networkEventRegisterAny.assert_not_called()
-- 
2.27.0


