vm: improve sending events for tags and features

1. Send the event after setting tag/feature
2. Provide old value for feature
3. Rename 'key' kwarg to 'feature'

qubes/tests/vm/init.py

@@ -219,21 +219,22 @@ class TC_21_Features(qubes.tests.QubesTestCase):
     def test_000_set(self):
         self.features['testfeature'] = 'value'
         self.assertEventFired(self.vm, 'domain-feature-set',
-            kwargs={'key': 'testfeature', 'value': 'value'})
+            kwargs={'feature': 'testfeature', 'value': 'value'})
 
     def test_001_set_existing(self):
-        self.features['test'] = 'value'
+        self.features['test'] = 'oldvalue'
         self.vm.fired_events.clear()
         self.features['test'] = 'value'
         self.assertEventFired(self.vm, 'domain-feature-set',
-            kwargs={'key': 'test', 'value': 'value'})
+            kwargs={'feature': 'test', 'value': 'value', 'oldvalue':
+                'oldvalue'})
 
     def test_002_unset(self):
         self.features['test'] = 'value'
         self.vm.fired_events.clear()
         del self.features['test']
         self.assertEventFired(self.vm, 'domain-feature-delete',
-            kwargs={'key': 'test'})
+            kwargs={'feature': 'test'})
 
     def test_003_unset_not_present(self):
         with self.assertRaises(KeyError):
@@ -244,18 +245,18 @@ class TC_21_Features(qubes.tests.QubesTestCase):
         self.features['test'] = True
         self.assertTrue(self.features['test'])
         self.assertEventFired(self.vm, 'domain-feature-set',
-            kwargs={'key': 'test', 'value': '1'})
+            kwargs={'feature': 'test', 'value': '1'})
 
     def test_005_set_bool_false(self):
         self.features['test'] = False
         self.assertFalse(self.features['test'])
         self.assertEventFired(self.vm, 'domain-feature-set',
-            kwargs={'key': 'test', 'value': ''})
+            kwargs={'feature': 'test', 'value': ''})
 
     def test_006_set_int(self):
         self.features['test'] = 123
         self.assertEventFired(self.vm, 'domain-feature-set',
-            kwargs={'key': 'test', 'value': '123'})
+            kwargs={'feature': 'test', 'value': '123'})
 
     def test_007_clear(self):
         self.features['test'] = 'value1'
@@ -263,9 +264,9 @@ class TC_21_Features(qubes.tests.QubesTestCase):
         self.vm.fired_events.clear()
         self.features.clear()
         self.assertEventFired(self.vm, 'domain-feature-delete',
-            kwargs={'key': 'test'})
+            kwargs={'feature': 'test'})
         self.assertEventFired(self.vm, 'domain-feature-delete',
-            kwargs={'key': 'test2'})
+            kwargs={'feature': 'test2'})
 
     def test_008_update(self):
         self.features['test'] = 'value'
@@ -276,6 +277,7 @@ class TC_21_Features(qubes.tests.QubesTestCase):
         self.assertEqual(self.features['test3'], 'value4')
         self.assertEqual(self.features['test'], 'value')
         self.assertEventFired(self.vm, 'domain-feature-set',
-            kwargs={'key': 'test2', 'value': 'value3'})
+            kwargs={'feature': 'test2', 'value': 'value3',
+                'oldvalue': 'value2'})
         self.assertEventFired(self.vm, 'domain-feature-set',
-            kwargs={'key': 'test3', 'value': 'value4'})
+            kwargs={'feature': 'test3', 'value': 'value4'})

qubes/vm/__init__.py

@@ -95,17 +95,24 @@ class Features(dict):
 
     def __delitem__(self, key):
         super(Features, self).__delitem__(key)
-        self.vm.fire_event('domain-feature-delete', key=key)
+        self.vm.fire_event('domain-feature-delete', feature=key)
 
     def __setitem__(self, key, value):
         if value is None or isinstance(value, bool):
             value = '1' if value else ''
         else:
             value = str(value)
-        # TODO: perhaps this shouldn't be fired on unchanged value? or at
-        # least oldvalue should be provided?
-        self.vm.fire_event('domain-feature-set', key=key, value=value)
+        try:
+            oldvalue = self[key]
+            has_oldvalue = True
+        except KeyError:
+            has_oldvalue = False
         super(Features, self).__setitem__(key, value)
+        if has_oldvalue:
+            self.vm.fire_event('domain-feature-set', feature=key, value=value,
+                oldvalue=oldvalue)
+        else:
+            self.vm.fire_event('domain-feature-set', feature=key, value=value)
 
     def clear(self):
         for key in tuple(self):
@@ -225,8 +232,8 @@ class Tags(set):
             raise ValueError('Invalid character in tag')
         if elem in self:
             return
-        self.vm.fire_event('domain-tag-add', tag=elem)
         super(Tags, self).add(elem)
+        self.vm.fire_event('domain-tag-add', tag=elem)
 
     def remove(self, elem):
         '''Remove a tag'''

qubes/vm/qubesvm.py

@@ -276,14 +276,17 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
             :param subject: Event emitter (the qube object)
             :param event: Event name (``'domain-restore'``)
 
-        .. event:: domain-feature-set (subject, event, key, value)
+        .. event:: domain-feature-set (subject, event, feature, value
+            [, oldvalue])
 
             A feature was changed.
+            *oldvalue* is present only when there was any.
 
             :param subject: Event emitter (the qube object)
             :param event: Event name (``'domain-feature-set'``)
-            :param key: feature name
+            :param feature: feature name
             :param value: new value
+            :param oldvalue: old value, if any
 
         .. event:: domain-feature-delete (subject, event, key)
 
@@ -291,7 +294,7 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
 
             :param subject: Event emitter (the qube object)
             :param event: Event name (``'domain-feature-delete'``)
-            :param key: feature name
+            :param feature: feature name
 
         .. event:: domain-tag-add (subject, event, tag)