Add method to solely create disposable#401
Conversation
qubesadmin/vm/__init__.py
Outdated
|
|
||
| wrapper = DispVMWrapper(app, method_dest) | ||
| method_dest = "dom0" | ||
| dispvm = app.qubesd_call(method_dest, "admin.vm.CreateDisposable") |
There was a problem hiding this comment.
This is a bad idea. Normal qubes intentionally have no permission to call admin.vm.CreateDisposable (nor any other admin.* calls), but they may be granted calls to the @dispvm target (possibly choosing different disposable template depending on the requested service).
Requiring extended admin API access just to measure time a bit better is no-go...
There was a problem hiding this comment.
Background:
commit 9bb59cdd20fa551563a04346559be2f09500708a
Author: Marek Marczykowski-Górecki <[email protected]>
Date: Sun Aug 6 12:22:47 2017 +0200
vm: add DispVMWrapper for calling a single service in new DispVM
This is a wrapper to use `$dispvm` target of qrexec call, just like any
other service call in qubesadmin module - using vm.run_service().
When running in dom0, qrexec-client-vm is not available, so DispVM needs
to be created "manually", using appropriate Admin API call
(admin.vm.CreateDisposable).
QubesOS/qubes-issues#2974
I did not comprehend why using $dispvm was necessary, thought it served the necessity at that time only.
This is a bad idea. Normal qubes intentionally have no permission to call
admin.vm.CreateDisposable(nor any otheradmin.*calls), but they may be granted calls to the@dispvmtarget (possibly choosing different disposable template depending on the requested service).
I'm not sure I completely understand this. What you are saying is that this is problematic when core-admin-client is running on a domU?
Requiring extended admin API access just to measure time a bit better is no-go...
What about a using the old code, maintaining the use of $dispvm*, and adding method just to create the disposable if self._method_dest.startswith("$dispvm")?
class DispVMWrapper(QubesVM):
"""Wrapper class for new DispVM, supporting only service call
Note that when running in dom0, one need to manually kill the DispVM after
service call ends.
"""
def create_disposable(self):
"""Create dispvm at service call or run service directly."""
if self._method_dest.startswith("$dispvm"):
if self._method_dest.startswith("$dispvm:"):
method_dest = self._method_dest[len("$dispvm:") :]
else:
method_dest = "dom0"
dispvm = self.app.qubesd_call(
method_dest, "admin.vm.CreateDisposable"
)
dispvm = dispvm.decode("ascii")
self._method_dest = dispvm
return self
def run_service(self, service, **kwargs):
"""Create disposable if absent and run service."""
if (
self.app.qubesd_connection_type == "socket"
and self._method_dest.startswith("$dispvm")
):
self.create_disposable()
# Service call may wait for session start, give it more time
# than default 5s
kwargs["connect_timeout"] = self.qrexec_timeout
return super().run_service(service, **kwargs)
def cleanup(self):
"""Cleanup after disposable usage."""
# in 'remote' case nothing is needed, as DispVM is cleaned up
# automatically
if (
self.app.qubesd_connection_type == "socket"
and not self._method_dest.startswith("$dispvm")
):
try:
self.kill()
except qubesadmin.exc.QubesVMNotRunningError:
pass
def start(self):
"""Create disposable if absent and start it."""
if self._method_dest.startswith("$dispvm"):
self.create_disposable()
super().start()
class DispVM(QubesVM):
"""Disposable VM"""
@classmethod
def from_appvm(cls, app, appvm):
"""Returns a wrapper for calling service in a new DispVM based on given
AppVM. If *appvm* is none, use default DispVM template"""
if appvm:
method_dest = "$dispvm:" + str(appvm)
else:
method_dest = "$dispvm"
wrapper = DispVMWrapper(app, method_dest)
return wrapperThere was a problem hiding this comment.
The solution above allows:
dispvm = qubesadmin.vm.DispVM.from_appvm(app, appvm).create_disposable()
There was a problem hiding this comment.
What you are saying is that this is problematic when core-admin-client is running on a domU?
Yes.
What about a using the old code, maintaining the use of
$dispvm*, and adding method just to create the disposableif self._method_dest.startswith("$dispvm")?
This approach is okay, it will preserve using special dispvm target, but will allow explicitly created disposable if somebody really wants.
BTW, while at it, it would be good to change deprecated $dispvm to @dispvm.
Generation cannot help on "from_appvm()" as normal qubes don't have permission to call "admin.vm.CreateDisposable" or other "admin.*" calls. With this new method, retrieval of the qube object can be done directly: dispvm = qubesadmin.vm.DispVM.from_appvm(app, dvm).create_disposable() For: QubesOS/qubes-issues#10230 For: QubesOS/qubes-issues#1512
Itches that it is different.
ben-grande
force-pushed
the
preload-optimize
branch
from
22df93b to
96e818a
Compare
ben-grande
changed the title
|
PipelineRetryFailed |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #401 +/- ##
==========================================
+ Coverage 76.10% 76.15% +0.04%
==========================================
Files 53 53
Lines 9287 9285 -2
==========================================
+ Hits 7068 7071 +3
+ Misses 2219 2214 -5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Openqa me please. |
OpenQA test summaryComplete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2025121808-4.3&flavor=pull-requests Test run included the following:
New failures, excluding unstableCompared to: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2025111104-4.3&flavor=update Failed testsNo failures! Fixed failuresCompared to: https://openqa.qubes-os.org/tests/158999#dependencies 10 fixed
Unstable testsDetailsPerformance TestsPerformance degradation:No issues Remaining performance tests:No remaining performance tests |
3 participants
In line with the qubes module, disposable creation happens on from_appvm(). This change is intended to measure how much time it takes to get a fresh disposable object, improving the performance tests distinction of the from_appvm() phase from the execution/run*() phase.
For: QubesOS/qubes-issues#10230
For: QubesOS/qubes-issues#1512