-
-
Notifications
You must be signed in to change notification settings - Fork 106
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add vm.icon property #342
Add vm.icon property #342
Conversation
This is a property for handling vm icons that change depending on vm type. Depends on QubesOS/qubes-artwork#17 references QubesOS/qubes-issues#5767
Codecov Report
@@ Coverage Diff @@
## master #342 +/- ##
==========================================
+ Coverage 63.58% 63.94% +0.35%
==========================================
Files 50 49 -1
Lines 8978 9036 +58
==========================================
+ Hits 5709 5778 +69
+ Misses 3269 3258 -11
Continue to review full report at Codecov.
|
A fossil from Qubes 3.2 times.
@qubes.ext.handler('property-del:provides_network') | ||
def on_property_del(self, subject, event, name): | ||
# pylint: disable=unused-argument | ||
self.set_servicevm_feature(subject) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This all will work fine for new VMs, because default value for provides_network is false (so default not having servicevm
feature is correct).
But the above miss adding the feature for existing VMs (on system update from the version before this change). The easiest way to do that is adding similar handler for domain-load
event (loading a domain from qubes.xml - basically qubesd service start).
qubes/ext/core_features.py
Outdated
del subject.features['servicevm'] | ||
|
||
@qubes.ext.handler('property-set:provides_network') | ||
def on_property_set(self, subject, _event, _name, _newvalue, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This won't work - event's arguments are keyword arguments so must have specific names (see tests that fail now).
qubes/ext/core_features.py
Outdated
self.set_servicevm_feature(subject) | ||
|
||
@qubes.ext.handler('domain-load') | ||
def on_property_del(self, subject, _event): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicated function name (see pylint).
Used by vm.icon method, at the moment features['servicevm'] is set when a VM provides_network.
if getattr(subject, 'provides_network', False): | ||
subject.features['servicevm'] = 1 | ||
elif 'servicevm' in subject.features: | ||
del subject.features['servicevm'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was deleting servicevm
necessary? Later on this commit:
Came here from QubesOS/qubes-mgmt-salt-dom0-virtual-machines#63
def test_100_servicevm_feature(self):
self.vm.provides_network = True
self.ext.set_servicevm_feature(self.vm)
self.assertEqual(self.features['servicevm'], 1)
self.vm.provides_network = False
self.ext.set_servicevm_feature(self.vm)
self.assertNotIn('servicevm', self.features)
I don't think not having provides_network
must enforce not having servicevm
.
This is a property for handling vm icons that change depending on vm type.
Depends on QubesOS/qubes-artwork#17
references QubesOS/qubes-issues#5767