From 218d43a2e0adf307428e3d3563be8ad826835b0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Tue, 3 Dec 2019 02:59:09 +0100 Subject: [PATCH] Add simple properties caching Reduce Admin API calls by caching returned values. The cache is not enabled by default, because it could result in stale values being returned. It can be enabled by setting 'cache_enabled' to True on Qubes() object. This is safe in two cases: - the application don't care about changed values - like a short-lived process that retrieve values once (for example qvm-ls) - the application listen for events and invalidate cache when property is changed For the second case, invalidating the cache on appropriate event (property-set:*, property-reset:*) is done before calling other event handlers. This is because the event may try to access the property value (not necessary from the event arguments), so we need to be sure it will see the new value. Fixes QubesOS/qubes-issues#5415 --- qubesadmin/app.py | 31 ++++++++++++++++++++++ qubesadmin/base.py | 49 ++++++++++++++++++++++++++++------- qubesadmin/events/__init__.py | 9 ++++++- 3 files changed, 79 insertions(+), 10 deletions(-) diff --git a/qubesadmin/app.py b/qubesadmin/app.py index 71d0902..a49c5db 100644 --- a/qubesadmin/app.py +++ b/qubesadmin/app.py @@ -153,6 +153,8 @@ class QubesBase(qubesadmin.base.PropertyHolder): log = None #: do not check for object (VM, label etc) existence before really needed blind_mode = False + #: cache retrieved properties values + cache_enabled = False def __init__(self): super(QubesBase, self).__init__(self, 'admin.property.', 'dom0') @@ -576,6 +578,35 @@ class QubesBase(qubesadmin.base.PropertyHolder): stdout, stderr = proc.communicate() return proc, stdout, stderr + def _invalidate_cache(self, subject, event, name, **kwargs): + """Invalidate cached value of a property. + + This method is designed to be hooked as an event handler for: + - property-set:* + - property-del:* + + This is done in :py:class:`qubesadmin.events.EventsDispatcher` class + directly, before calling other handlers. + + It handles both VM and global properties. + Note: even if the new value is given in the event arguments, it is + ignored because it comes without type information. + + :param subject: either VM object or None + :param event: name of the event + :param name: name of the property + :param kwargs: other arguments + :return: none + """ # pylint: disable=unused-argument + if subject is None: + subject = self + + try: + # pylint: disable=protected-access + del subject._properties_cache[name] + except KeyError: + pass + class QubesLocal(QubesBase): """Application object communicating through local socket. diff --git a/qubesadmin/base.py b/qubesadmin/base.py index 2a8fcaa..1b1d6f0 100644 --- a/qubesadmin/base.py +++ b/qubesadmin/base.py @@ -44,6 +44,7 @@ class PropertyHolder(object): self._method_dest = method_dest self._properties = None self._properties_help = None + self._properties_cache = {} def qubesd_call(self, dest, method, arg=None, payload=None, payload_stream=None): @@ -141,16 +142,20 @@ class PropertyHolder(object): ''' if item.startswith('_'): raise AttributeError(item) + # cached value + if item in self._properties_cache: + return self._properties_cache[item][0] + # cached properties list + if self._properties is not None and item not in self._properties: + raise AttributeError(item) property_str = self.qubesd_call( self._method_dest, self._method_prefix + 'Get', item, None) - (default, _value) = property_str.split(b' ', 1) - assert default.startswith(b'default=') - is_default_str = default.split(b'=')[1] - is_default = is_default_str.decode('ascii') == "True" - assert isinstance(is_default, bool) + is_default, value = self._deserialize_property(property_str) + if self.app.cache_enabled: + self._properties_cache[item] = (is_default, value) return is_default def property_get_default(self, item): @@ -192,6 +197,15 @@ class PropertyHolder(object): def __getattr__(self, item): if item.startswith('_'): raise AttributeError(item) + # cached value + if item in self._properties_cache: + value = self._properties_cache[item][1] + if value is AttributeError: + raise AttributeError(item) + return value + # cached properties list + if self._properties is not None and item not in self._properties: + raise AttributeError(item) try: property_str = self.qubesd_call( self._method_dest, @@ -200,8 +214,25 @@ class PropertyHolder(object): None) except qubesadmin.exc.QubesDaemonNoResponseError: raise qubesadmin.exc.QubesPropertyAccessError(item) - (_default, prop_type, value) = property_str.split(b' ', 2) - return self._parse_type_value(prop_type, value) + is_default, value = self._deserialize_property(property_str) + if self.app.cache_enabled: + self._properties_cache[item] = (is_default, value) + if value is AttributeError: + raise AttributeError(item) + return value + + def _deserialize_property(self, api_response): + """ + Deserialize property.Get response format + :param api_response: bytes, as retrieved from qubesd + :return: tuple(is_default, value) + """ + (default, prop_type, value) = api_response.split(b' ', 2) + assert default.startswith(b'default=') + is_default_str = default.split(b'=')[1] + is_default = is_default_str.decode('ascii') == "True" + value = self._parse_type_value(prop_type, value) + return is_default, value def _parse_type_value(self, prop_type, value): ''' @@ -224,11 +255,11 @@ class PropertyHolder(object): return str(value) if prop_type == 'bool': if value == '': - raise AttributeError + return AttributeError return value == "True" if prop_type == 'int': if value == '': - raise AttributeError + return AttributeError return int(value) if prop_type == 'vm': if value == '': diff --git a/qubesadmin/events/__init__.py b/qubesadmin/events/__init__.py index 647c927..634c5db 100644 --- a/qubesadmin/events/__init__.py +++ b/qubesadmin/events/__init__.py @@ -188,7 +188,8 @@ class EventsDispatcher(object): return some_event_received def handle(self, subject, event, **kwargs): - '''Call handlers for given event''' + """Call handlers for given event""" + # pylint: disable=protected-access if subject: if event in ['property-set:name']: self.app.domains.clear_cache() @@ -210,6 +211,12 @@ class EventsDispatcher(object): .devices[devclass][ident] except (KeyError, ValueError): pass + # invalidate cache if needed; call it before other handlers + # as those may want to use cached value + if event.startswith('property-set:') or \ + event.startswith('property-reset:'): + self.app._invalidate_cache(subject, event, **kwargs) + handlers = [h_func for h_name, h_func_set in self.handlers.items() for h_func in h_func_set if fnmatch.fnmatch(event, h_name)]