Skip to content

Commit

Permalink
q-dev: persistent -> required
Browse files Browse the repository at this point in the history
  • Loading branch information
piotrbartman committed May 26, 2024
1 parent e5a4420 commit 5f7003d
Show file tree
Hide file tree
Showing 11 changed files with 107 additions and 88 deletions.
10 changes: 9 additions & 1 deletion doc/manpages/qvm-device.rst
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,17 @@ Attach the device with *DEVICE_ID* from *BACKEND_DOMAIN* to the domain *VMNAME*
Alias for the `read-only=yes` option. If you specify both `--ro` and
`--option read-only=no`, `--ro` takes precedence.

.. option:: --required, -r

Assign device persistently which means it will be required to the qube's startup and then automatically attached.

.. option:: --persistent, -p

Attach device persistently, which means have it attached also after qube restart.
Alias for `--required` for backward compatibility.

.. option:: --auto-attach, -a

Assign the device to a qube. It will be automatically attached upon the qube's startup or connection. The device will not be automatically attached if it has been manually detached or is already attached to another qube.

aliases: a, at

Expand Down
4 changes: 2 additions & 2 deletions qubesadmin/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -543,12 +543,12 @@ def clone_vm(self, src_vm, new_name, new_cls=None, pool=None, pools=None,
try:
for devclass in src_vm.devices:
for assignment in src_vm.devices[devclass].assignments(
persistent=True):
required=True):
new_assignment = qubesadmin.devices.DeviceAssignment(
backend_domain=assignment.backend_domain,
ident=assignment.ident,
options=assignment.options,
persistent=assignment.persistent)
required=assignment.required)
dst_vm.devices[devclass].attach(new_assignment)
except qubesadmin.exc.QubesException:
if not ignore_errors:
Expand Down
2 changes: 1 addition & 1 deletion qubesadmin/backup/restore.py
Original file line number Diff line number Diff line change
Expand Up @@ -2091,7 +2091,7 @@ def _restore_vms_metadata(self, restore_info):
backend_domain=backend_domain,
ident=ident,
options=options,
persistent=True)
required=True)
try:
if not self.options.verify_only:
new_vm.devices[bus].attach(assignment)
Expand Down
65 changes: 32 additions & 33 deletions qubesadmin/devices.py
Original file line number Diff line number Diff line change
Expand Up @@ -552,25 +552,25 @@ def __init__(self, backend_domain, devclass, ident, **kwargs):
class DeviceAssignment(Device):
""" Maps a device to a frontend_domain. """

def __init__(self, backend_domain, ident, options=None, persistent=False,
def __init__(self, backend_domain, ident, options=None,
frontend_domain=None, devclass=None,
required=False, attach_automatically=False):
super().__init__(backend_domain, ident, devclass)
self.__options = options or {}
self.persistent = persistent
self.__required = required
self.__attach_automatically = attach_automatically
self.__frontend_domain = frontend_domain

def clone(self):
"""Clone object instance"""
return self.__class__(
self.backend_domain,
self.ident,
self.options,
self.persistent,
self.frontend_domain,
self.devclass,
backend_domain=self.backend_domain,
ident=self.ident,
options=self.options,
required=self.required,
attach_automatically=self.attach_automatically,
frontend_domain=self.frontend_domain,
devclass=self.devclass,
)

@property
Expand All @@ -595,28 +595,24 @@ def required(self) -> bool:
"""
Is the presence of this device required for the domain to start? If yes,
it will be attached automatically.
TODO: this possibly should not be available for usb device? or always False?
TODO: this is a reworking of the previously existing "persistent" attachment, split in two option
"""
return self.persistent # TODO
return self.__required

@required.setter
def required(self, required: bool):
self.persistent = required # TODO
self.__required = required

@property
def attach_automatically(self) -> bool:
"""
Should this device automatically connect to the frontend domain when
available and not connected to other qubes?
TODO: this possibly should not be available for usb device? or always False?
TODO: this is a reworking of the previously existing "persistent" attachment, split in two option
"""
return self.persistent # TODO
return self.__attach_automatically

@attach_automatically.setter
def attach_automatically(self, attach_automatically: bool):
self.persistent = attach_automatically # TODO
self.__attach_automatically = attach_automatically

@property
def options(self) -> Dict[str, Any]:
Expand Down Expand Up @@ -663,8 +659,10 @@ def attach(self, device_assignment):
f"{device_assignment.devclass=}!={self._class=}")

options = device_assignment.options.copy()
if device_assignment.persistent:
options['persistent'] = 'True'
if device_assignment.required:
options['required'] = 'True'
# if device_assignment.attach_automatically:
# options['attach_automatically'] = 'True'
options_str = ' '.join('{}={}'.format(opt, val)
for opt, val in sorted(options.items()))
self._vm.qubesd_call(None,
Expand Down Expand Up @@ -698,16 +696,17 @@ def detach(self, device_assignment):
device_assignment.backend_domain,
device_assignment.ident))

def assignments(self, persistent=None):
def assignments(self, required=None):
"""List assignments for devices which are (or may be) attached to the
vm.
# TODO: handle auto-attach
Devices may be attached persistently (so they are included in
:file:`qubes.xml`) or not. Device can also be in :file:`qubes.xml`,
but be temporarily detached.
:param bool persistent: only include devices which are or are not
attached persistently.
:param bool required: only include devices which are or are not
required to start qube.
"""

assignments_str = self._vm.qubesd_call(None,
Expand All @@ -719,28 +718,28 @@ def assignments(self, persistent=None):
options = dict(opt_single.split('=', 1)
for opt_single in options_all.split(' ') if
opt_single)
dev_persistent = (options.pop('persistent', False) in
dev_required = (options.pop('required', False) in
['True', 'yes', True])
if persistent is not None and dev_persistent != persistent:
dev_auto_attach = (options.pop('attach_automatically', False) in
['True', 'yes', True]) # TODO
if required is not None and dev_required != required:
continue
backend_domain = self._vm.app.domains.get_blind(backend_domain)
yield DeviceAssignment(backend_domain, ident, options,
persistent=dev_persistent,
required=dev_required,
frontend_domain=self._vm,
devclass=self._class)

def attached(self):
"""List devices which are (or may be) attached to this vm """

for assignment in self.assignments():
yield assignment.device

def persistent(self):
def required(self):
""" Devices persistently attached and safe to access before libvirt
bootstrap.
"""

for assignment in self.assignments(True):
for assignment in self.assignments(required=True):
yield assignment.device

def available(self):
Expand All @@ -754,19 +753,19 @@ def available(self):
expected_devclass=self._class,
)

def update_persistent(self, device, persistent):
"""Update `persistent` flag of already attached device.
def update_required(self, device: DeviceInfo, required: bool): # TODO: update auto-attach
"""Update `required` flag of already attached device.
:param DeviceInfo device: device for which change persistent flag
:param bool persistent: new persistent flag
:param DeviceInfo device: device for which change required flag
:param bool required: new required flag
"""

self._vm.qubesd_call(None,
'admin.vm.device.{}.Set.persistent'.format(
self._class),
'{!s}+{!s}'.format(device.backend_domain,
device.ident),
str(persistent).encode('utf-8'))
str(required).encode('utf-8'))

__iter__ = available

Expand Down
8 changes: 4 additions & 4 deletions qubesadmin/tests/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -736,11 +736,11 @@ def test_043_clone_devices(self):
self.app.expected_calls[
('test-vm', 'admin.vm.device.pci.List', None, None)] = \
b'0\0test-vm2+dev1 ro=True\n' \
b'test-vm3+dev2 persistent=True\n'
b'test-vm3+dev2 required=True\n'

self.app.expected_calls[
('new-name', 'admin.vm.device.pci.Attach', 'test-vm3+dev2',
b'persistent=True')] = b'0\0'
b'required=True')] = b'0\0'

new_vm = self.app.clone_vm('test-vm', 'new-name')
self.assertEqual(new_vm.name, 'new-name')
Expand Down Expand Up @@ -772,11 +772,11 @@ def test_044_clone_devices_fail(self):
self.app.expected_calls[
('test-vm', 'admin.vm.device.pci.List', None, None)] = \
b'0\0test-vm2+dev1 ro=True\n' \
b'test-vm3+dev2 persistent=True\n'
b'test-vm3+dev2 required=True\n'

self.app.expected_calls[
('new-name', 'admin.vm.device.pci.Attach', 'test-vm3+dev2',
b'persistent=True')] = \
b'required=True')] = \
b'2\0QubesException\0\0something happened\0'

self.app.expected_calls[
Expand Down
3 changes: 2 additions & 1 deletion qubesadmin/tests/backup/backupcompatibility.py
Original file line number Diff line number Diff line change
Expand Up @@ -1435,7 +1435,8 @@ def setup_expected_calls(self, parsed_qubes_xml, templates_map=None):
for bus, devices in vm['devices'].items():
for (backend_domain, ident), options in devices.items():
all_options = options.copy()
all_options['persistent'] = True
all_options['required'] = True
# all_options['attach_automatically'] = True # TODO
encoded_options = ' '.join('{}={}'.format(key, value) for
key, value in all_options.items()).encode()
self.app.expected_calls[
Expand Down
Loading

0 comments on commit 5f7003d

Please sign in to comment.