Skip to content

Commit

Permalink
q-dev: set.assignment -> set.required
Browse files Browse the repository at this point in the history
  • Loading branch information
piotrbartman committed Jun 12, 2024
1 parent 0193c0a commit af8996f
Show file tree
Hide file tree
Showing 8 changed files with 44 additions and 99 deletions.
10 changes: 5 additions & 5 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -66,31 +66,31 @@ ADMIN_API_METHODS_SIMPLE = \
admin.vm.device.pci.Attached \
admin.vm.device.pci.Available \
admin.vm.device.pci.Detach \
admin.vm.device.pci.Set.assignment \
admin.vm.device.pci.Set.required \
admin.vm.device.pci.Unassign \
admin.vm.device.block.Assign \
admin.vm.device.block.Assigned \
admin.vm.device.block.Attach \
admin.vm.device.block.Attached \
admin.vm.device.block.Available \
admin.vm.device.block.Detach \
admin.vm.device.block.Set.assignment \
admin.vm.device.block.Set.required \
admin.vm.device.block.Unassign \
admin.vm.device.usb.Assign \
admin.vm.device.usb.Assigned \
admin.vm.device.usb.Attach \
admin.vm.device.usb.Attached \
admin.vm.device.usb.Available \
admin.vm.device.usb.Detach \
admin.vm.device.usb.Set.assignment \
admin.vm.device.usb.Set.required \
admin.vm.device.usb.Unassign \
admin.vm.device.mic.Assign \
admin.vm.device.mic.Assigned \
admin.vm.device.mic.Attach \
admin.vm.device.mic.Attached \
admin.vm.device.mic.Available \
admin.vm.device.mic.Detach \
admin.vm.device.mic.Set.assignment \
admin.vm.device.mic.Set.required \
admin.vm.device.mic.Unassign \
admin.vm.feature.CheckWithNetvm \
admin.vm.feature.CheckWithTemplate \
Expand Down Expand Up @@ -227,7 +227,7 @@ endif
admin.vm.device.testclass.Unassign \
admin.vm.device.testclass.Attached \
admin.vm.device.testclass.Assigned \
admin.vm.device.testclass.Set.assignment \
admin.vm.device.testclass.Set.required \
admin.vm.device.testclass.Available
install -d $(DESTDIR)/etc/qubes/policy.d/include
install -m 0644 qubes-rpc-policy/admin-local-ro \
Expand Down
4 changes: 2 additions & 2 deletions qubes-rpc-policy/90-admin-default.policy.header
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,14 @@
!include-service admin.vm.device.mic.Attached * include/admin-local-ro
!include-service admin.vm.device.mic.Available * include/admin-local-ro
!include-service admin.vm.device.mic.Detach * include/admin-local-rwx
!include-service admin.vm.device.mic.Set.assignment * include/admin-local-rwx
!include-service admin.vm.device.mic.Set.required * include/admin-local-rwx
!include-service admin.vm.device.mic.Unassign * include/admin-local-rwx
!include-service admin.vm.device.usb.Assign * include/admin-local-rwx
!include-service admin.vm.device.usb.Assigned * include/admin-local-ro
!include-service admin.vm.device.usb.Attach * include/admin-local-rwx
!include-service admin.vm.device.usb.Attached * include/admin-local-ro
!include-service admin.vm.device.usb.Available * include/admin-local-ro
!include-service admin.vm.device.usb.Detach * include/admin-local-rwx
!include-service admin.vm.device.usb.Set.assignment * include/admin-local-rwx
!include-service admin.vm.device.usb.Set.required * include/admin-local-rwx
!include-service admin.vm.device.usb.Unassign * include/admin-local-rwx

11 changes: 5 additions & 6 deletions qubes/api/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -1370,22 +1370,21 @@ async def vm_device_detach(self, endpoint):

# Assign/Unassign action can modify only a persistent state of running VM.
# For this reason, write=True
@qubes.api.method('admin.vm.device.{endpoint}.Set.assignment',
@qubes.api.method('admin.vm.device.{endpoint}.Set.required',
endpoints=(ep.name
for ep in importlib.metadata.entry_points(group='qubes.devices')),
scope='local', write=True)
async def vm_device_set_assignment(self, endpoint, untrusted_payload):
async def vm_device_set_required(self, endpoint, untrusted_payload):
"""
Update assignment of an already assigned device.
Update `required` flag of an already assigned device.
Payload:
`None` -> unassign device from a qube
`False` -> device will be auto-attached to qube
`True` -> device is required to start qube
"""
devclass = endpoint

self.enforce(untrusted_payload in (b'True', b'False', b'None'))
self.enforce(untrusted_payload in (b'True', b'False'))
# now is safe to eval, since the value of untrusted_payload is trusted
# pylint: disable=eval-used
assignment = eval(untrusted_payload)
Expand All @@ -1398,7 +1397,7 @@ async def vm_device_set_assignment(self, endpoint, untrusted_payload):

self.fire_event_for_permission(device=dev, assignment=assignment)

await self.dest.devices[devclass].update_assignment(dev, assignment)
await self.dest.devices[devclass].update_required(dev, assignment)
self.app.save()

@qubes.api.method('admin.vm.firewall.Get', no_payload=True,
Expand Down
22 changes: 8 additions & 14 deletions qubes/devices.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
`device-list-change:class` event.
"""
import itertools
from typing import Optional, Iterable
from typing import Iterable

import qubes.exc
import qubes.utils
Expand Down Expand Up @@ -253,13 +253,12 @@ def load_assignment(self, device_assignment: DeviceAssignment):
device_assignment.devclass = self._bus
self._set.add(device_assignment)

async def update_assignment(self, device: Device, required: Optional[bool]):
async def update_required(self, device: Device, required: bool):
"""
Update assignment of an already attached device.
Update `required` flag of an already attached device.
:param Device device: device for which change required flag
:param bool required: new assignment:
`None` -> unassign device from qube
`False` -> device will be auto-attached to qube
`True` -> device is required to start qube
"""
Expand All @@ -278,17 +277,12 @@ async def update_assignment(self, device: Device, required: Optional[bool]):

# be careful to use already present assignment, not the provided one
# - to not change options as a side effect
if required is not None:
if assignment.required == required:
return
if assignment.required == required:
return

assignment.required = required
await self._vm.fire_event_async(
'device-assignment-changed:' + self._bus, device=device)
else:
await self._vm.fire_event_async(
'device-unassign:' + self._bus, device=device)
await self.unassign(assignment)
assignment.required = required
await self._vm.fire_event_async(
'device-assignment-changed:' + self._bus, device=device)

async def detach(self, device: Device):
"""
Expand Down
1 change: 0 additions & 1 deletion qubes/ext/block.py
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,6 @@ async def on_domain_start(self, vm, _event, **_kwargs):
vm, assignment.device, assignment.options)

def notify_auto_attached(self, vm, device, options):
pass
# bypass DeviceCollection logic preventing double attach
self.pre_attachment_internal(vm, device, options)

Expand Down
58 changes: 14 additions & 44 deletions qubes/tests/api_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -2831,7 +2831,7 @@ def test_642_vm_create_disposable_not_allowed(self, storage_mock):
b'test-vm1')
self.assertFalse(self.app.save.called)

def test_650_vm_device_set_assignment_true(self):
def test_650_vm_device_set_required_true(self):
assignment = qubes.device_protocol.DeviceAssignment(
self.vm, '1234', attach_automatically=True, required=False,
options={'opt1': 'value'})
Expand All @@ -2845,7 +2845,7 @@ def test_650_vm_device_set_assignment_true(self):
with unittest.mock.patch.object(qubes.vm.qubesvm.QubesVM,
'is_halted', lambda _: False):
value = self.call_mgmt_func(
b'admin.vm.device.testclass.Set.assignment',
b'admin.vm.device.testclass.Set.required',
b'test-vm1', b'test-vm1+1234', b'True')

self.assertIsNone(value)
Expand All @@ -2855,13 +2855,13 @@ def test_650_vm_device_set_assignment_true(self):
self.assertIn(dev, required)
self.assertEventFired(
self.emitter,
'admin-permission:admin.vm.device.testclass.Set.assignment')
'admin-permission:admin.vm.device.testclass.Set.required')
mock_action.assert_called_once_with(
self.vm, f'device-assignment-changed:testclass',
device=self.vm.devices['testclass']['1234'])
self.app.save.assert_called_once_with()

def test_651_vm_device_set_assignment_false(self):
def test_651_vm_device_set_required_false(self):
assignment = qubes.device_protocol.DeviceAssignment(
self.vm, '1234', attach_automatically=True, required=True,
options={'opt1': 'value'})
Expand All @@ -2875,7 +2875,7 @@ def test_651_vm_device_set_assignment_false(self):
with unittest.mock.patch.object(qubes.vm.qubesvm.QubesVM,
'is_halted', lambda _: False):
value = self.call_mgmt_func(
b'admin.vm.device.testclass.Set.assignment',
b'admin.vm.device.testclass.Set.required',
b'test-vm1', b'test-vm1+1234', b'False')

self.assertIsNone(value)
Expand All @@ -2885,43 +2885,13 @@ def test_651_vm_device_set_assignment_false(self):
self.assertNotIn(dev, required)
self.assertEventFired(
self.emitter,
'admin-permission:admin.vm.device.testclass.Set.assignment')
'admin-permission:admin.vm.device.testclass.Set.required')
mock_action.assert_called_once_with(
self.vm, f'device-assignment-changed:testclass',
device=self.vm.devices['testclass']['1234'])
self.app.save.assert_called_once_with()

def test_652_vm_device_set_assignment_none(self):
assignment = qubes.device_protocol.DeviceAssignment(
self.vm, '1234', attach_automatically=True, required=False,
options={'opt1': 'value'})
self.loop.run_until_complete(
self.vm.devices['testclass'].assign(assignment))
mock_action = unittest.mock.Mock()
mock_action.return_value = None
del mock_action._is_coroutine
self.vm.add_handler(f'device-unassign:testclass', mock_action)

with unittest.mock.patch.object(qubes.vm.qubesvm.QubesVM,
'is_halted', lambda _: False):
value = self.call_mgmt_func(
b'admin.vm.device.testclass.Set.assignment',
b'test-vm1', b'test-vm1+1234', b'None')

self.assertIsNone(value)
dev = qubes.device_protocol.DeviceInfo(self.vm, '1234')
required = self.vm.devices['testclass'].get_assigned_devices(
required_only=False)
self.assertNotIn(dev, required)
self.assertEventFired(
self.emitter,
'admin-permission:admin.vm.device.testclass.Set.assignment')
mock_action.assert_called_once_with(
self.vm, f'device-unassign:testclass',
device=self.vm.devices['testclass']['1234'])
self.app.save.assert_called_once_with()

def test_653_vm_device_set_assignment_true_unchanged(self):
def test_652_vm_device_set_required_true_unchanged(self):
assignment = qubes.device_protocol.DeviceAssignment(
self.vm, '1234', attach_automatically=True, required=True,
options={'opt1': 'value'})
Expand All @@ -2930,7 +2900,7 @@ def test_653_vm_device_set_assignment_true_unchanged(self):
with unittest.mock.patch.object(qubes.vm.qubesvm.QubesVM,
'is_halted', lambda _: False):
value = self.call_mgmt_func(
b'admin.vm.device.testclass.Set.assignment',
b'admin.vm.device.testclass.Set.required',
b'test-vm1', b'test-vm1+1234', b'True')
self.assertIsNone(value)
dev = qubes.device_protocol.DeviceInfo(self.vm, '1234')
Expand All @@ -2939,7 +2909,7 @@ def test_653_vm_device_set_assignment_true_unchanged(self):
self.assertIn(dev, required)
self.app.save.assert_called_once_with()

def test_654_vm_device_set_assignment_false_unchanged(self):
def test_653_vm_device_set_required_false_unchanged(self):
assignment = qubes.device_protocol.DeviceAssignment(
self.vm, '1234', attach_automatically=True, required=False,
options={'opt1': 'value'})
Expand All @@ -2948,7 +2918,7 @@ def test_654_vm_device_set_assignment_false_unchanged(self):
with unittest.mock.patch.object(qubes.vm.qubesvm.QubesVM,
'is_halted', lambda _: False):
value = self.call_mgmt_func(
b'admin.vm.device.testclass.Set.assignment',
b'admin.vm.device.testclass.Set.required',
b'test-vm1', b'test-vm1+1234', b'False')
self.assertIsNone(value)
dev = qubes.device_protocol.DeviceInfo(self.vm, '1234')
Expand All @@ -2957,28 +2927,28 @@ def test_654_vm_device_set_assignment_false_unchanged(self):
self.assertNotIn(dev, required)
self.app.save.assert_called_once_with()

def test_655_vm_device_set_persistent_not_assigned(self):
def test_654_vm_device_set_persistent_not_assigned(self):
self.vm.add_handler('device-list:testclass',
self.device_list_testclass)
with unittest.mock.patch.object(qubes.vm.qubesvm.QubesVM,
'is_halted', lambda _: False):
with self.assertRaises(qubes.exc.QubesValueError):
self.call_mgmt_func(
b'admin.vm.device.testclass.Set.assignment',
b'admin.vm.device.testclass.Set.required',
b'test-vm1', b'test-vm1+1234', b'True')
dev = qubes.device_protocol.DeviceInfo(self.vm, '1234')
self.assertNotIn(
dev, self.vm.devices['testclass'].get_assigned_devices())
self.assertFalse(self.app.save.called)

def test_656_vm_device_set_persistent_invalid_value(self):
def test_655_vm_device_set_persistent_invalid_value(self):
self.vm.add_handler('device-list:testclass',
self.device_list_testclass)
with unittest.mock.patch.object(qubes.vm.qubesvm.QubesVM,
'is_halted', lambda _: False):
with self.assertRaises(qubes.exc.PermissionDenied):
self.call_mgmt_func(
b'admin.vm.device.testclass.Set.assignment',
b'admin.vm.device.testclass.Set.required',
b'test-vm1', b'test-vm1+1234', b'maybe')
dev = qubes.device_protocol.DeviceInfo(self.vm, '1234')
self.assertNotIn(dev, self.vm.devices['testclass'].get_assigned_devices())
Expand Down
33 changes: 8 additions & 25 deletions qubes/tests/devices.py
Original file line number Diff line number Diff line change
Expand Up @@ -211,30 +211,13 @@ def test_020_update_required_to_false(self):
self.assertEqual(
{self.device}, set(self.collection.get_assigned_devices()))
self.loop.run_until_complete(
self.collection.update_assignment(self.device, False))
self.collection.update_required(self.device, False))
self.assertEqual(
{self.device}, set(self.collection.get_assigned_devices()))
self.assertEqual(
{self.device}, set(self.collection.get_attached_devices()))

def test_021_update_required_to_none(self):
self.assertEqual(set([]), set(self.collection.get_assigned_devices()))
self.assignment.required = False
self.loop.run_until_complete(self.collection.assign(self.assignment))
self.attach()
self.assertEqual(
set(),
set(self.collection.get_assigned_devices(required_only=True)))
self.assertEqual(
{self.device}, set(self.collection.get_assigned_devices()))
self.loop.run_until_complete(
self.collection.update_assignment(self.device, None))
self.assertEqual(
set(), set(self.collection.get_assigned_devices()))
self.assertEqual(
{self.device}, set(self.collection.get_attached_devices()))

def test_022_update_required_to_true(self):
def test_021_update_required_to_true(self):
self.assignment.required = False
self.attach()
self.assertEqual(set(), set(self.collection.get_assigned_devices()))
Expand All @@ -249,32 +232,32 @@ def test_022_update_required_to_true(self):
self.assertEqual({self.device},
set(self.collection.get_attached_devices()))
self.loop.run_until_complete(
self.collection.update_assignment(self.device, True))
self.collection.update_required(self.device, True))
self.assertEqual({self.device},
set(self.collection.get_assigned_devices()))
self.assertEqual({self.device},
set(self.collection.get_attached_devices()))

def test_023_update_required_reject_not_running(self):
def test_022_update_required_reject_not_running(self):
self.assertEqual(set([]), set(self.collection.get_assigned_devices()))
self.loop.run_until_complete(self.collection.assign(self.assignment))
self.assertEqual({self.device},
set(self.collection.get_assigned_devices()))
self.assertEqual(set(), set(self.collection.get_attached_devices()))
with self.assertRaises(qubes.exc.QubesVMNotStartedError):
self.loop.run_until_complete(
self.collection.update_assignment(self.device, False))
self.collection.update_required(self.device, False))

def test_024_update_required_reject_not_attached(self):
def test_023_update_required_reject_not_attached(self):
self.assertEqual(set(), set(self.collection.get_assigned_devices()))
self.assertEqual(set(), set(self.collection.get_attached_devices()))
self.emitter.running = True
with self.assertRaises(qubes.exc.QubesValueError):
self.loop.run_until_complete(
self.collection.update_assignment(self.device, True))
self.collection.update_required(self.device, True))
with self.assertRaises(qubes.exc.QubesValueError):
self.loop.run_until_complete(
self.collection.update_assignment(self.device, False))
self.collection.update_required(self.device, False))

def test_030_assign(self):
self.emitter.running = True
Expand Down
4 changes: 2 additions & 2 deletions rpm_spec/core-dom0.spec.in
Original file line number Diff line number Diff line change
Expand Up @@ -260,15 +260,15 @@ admin.vm.device.block.Attach
admin.vm.device.block.Attached
admin.vm.device.block.Available
admin.vm.device.block.Detach
admin.vm.device.block.Set.assignment
admin.vm.device.block.Set.required
admin.vm.device.block.Unassign
admin.vm.device.pci.Assign
admin.vm.device.pci.Assigned
admin.vm.device.pci.Attach
admin.vm.device.pci.Attached
admin.vm.device.pci.Available
admin.vm.device.pci.Detach
admin.vm.device.pci.Set.assignment
admin.vm.device.pci.Set.required
admin.vm.device.pci.Unassign
admin.vm.feature.CheckWithAdminVM
admin.vm.feature.CheckWithNetvm
Expand Down

0 comments on commit af8996f

Please sign in to comment.