Add disposable preload setting to advanced settings by ben-grande · Pull Request #425 · QubesOS/qubes-manager

ben-grande · 2025-05-22T12:39:29Z

marmarta

One black thing - while I do not love it, apparently we are to be using it, so could you put in an option to revert to default 88 characters per line? Marek approved it, and it really cuts down on black being absolutely ridiculous. I do it in this PR: https://github.com/QubesOS/qubes-desktop-linux-manager/pull/258/files#diff-037ea159eb0a7cb0ac23b851e66bee30fb838ee8d0d99fa331a1ba65283d37f7

marmarta · 2025-05-23T14:21:27Z

        vm_memory = getattr(self.vm, "memory", None)
        vm_maxmem = getattr(self.vm, "maxmem", None)
+        vm_preload_dispvm = int(
+            self.vm.features.get("preload-dispvm-max", 0) or 0


I'm a little bit confused about this, why is there or 0 here?

The , 0 is for when there is no value, the or 0 is for when the value is empty ''. I need to use the , 0 to not fail the .get() if there is no value to retrieve.

I checked again, the get(..., 0) is not necessary if there is or 0. It also doesn't fail the get() method without it. The or 0 covers more cases.

marmarta · 2025-05-23T14:21:56Z

    <x>0</x>
    <y>0</y>
-    <width>836</width>
+    <width>858</width>


I think this is accidental?

I can revert this and other settings.

marmarta · 2025-05-23T14:22:23Z

        <locale language="English" country="UnitedStates"/>
       </property>
       <property name="currentIndex">
-        <number>0</number>


also accidental?

Not on purpose, see comment below.

marmarta · 2025-05-23T14:22:34Z

             <widget class="QLabel" name="type_label">
              <property name="font">
               <font>
-                <weight>50</weight>


why this and other font changes?

Not on purpose, changed automatically by Qt Designer 6.4.2 on Debian, maybe I need to switch to Fedora to get the 6.9.0... https://packages.fedoraproject.org/pkgs/qt6-qttools/qt6-designer/.

marmarta · 2025-05-23T14:25:10Z

                QtCore.QRegularExpression(
                    "[a-zA-Z0-9_-]*",
-                    QtCore.QRegularExpression.PatternOption.CaseInsensitiveOption,
+                    # fmt: off


why turning off formatting in such an awkward place (the middle of function declaration)? I do not like black myself, but I don't think this is a good approach to deal with its... opinions

I used black -l80, with -l88, this is possibly unnecessary..

marmarta · 2025-05-23T14:25:57Z


        vm_memory = getattr(self.vm, "memory", None)
        vm_maxmem = getattr(self.vm, "maxmem", None)
-        vm_preload_dispvm = int(


I think the preload-disposables changes should all be in one commit (and the qrexec and black stuff in their own commits)

Yes, that is just for easier review and reverting if some tests show regression. By the end, it will be squashed.

marmarta · 2025-05-23T14:35:02Z

+        else:
+            vm_preload_dispvm = 0
        self.preload_dispvm.setMinimum(0)
+        self.preload_dispvm.setMaximum(9999)


That does not seem like a sensible maximum... (Also, max and min can be set in the ui file; easiest way to edit it is with designer-qt)

Also set on the UI. I don't know a sensible maximum for this, the user can go crazy but the system won't preload if there is not enough memory. Is 50 a reasonable amount? I don't want to discourage users with a lot of memory in the future to not use the UI and use the terminal.

50 sounds like a reasonable amount, yeah

marmarta · 2025-05-23T14:36:31Z

+              <item row="6" column="0">
+               <widget class="QLabel" name="preload_dispvm_label">
+                <property name="text">
+                 <string>Preload disposables:</string>


I think this needs an explaining tooltip that clarifies what this setting does, exactly.

ben-grande · 2025-05-23T16:24:10Z

One black thing

I though Qubes project was using -l80.... I formatted all my PRs with it. At least qubes-core-* is using it. I will format things on the manager and linux-desktop-manager with -l88 them.

ben-grande · 2025-05-23T17:14:59Z

Now I used black -l88.

codecov · 2025-06-03T12:05:03Z

Codecov Report

Attention: Patch coverage is 77.57576% with 37 lines in your changes missing coverage. Please review.

Project coverage is 69.19%. Comparing base (bc659eb) to head (16f7bd0).
Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
qubesmanager/settings.py	77.57%	37 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #425      +/-   ##
==========================================
+ Coverage   69.08%   69.19%   +0.11%     
==========================================
  Files          17       17              
  Lines        3891     3909      +18     
==========================================
+ Hits         2688     2705      +17     
- Misses       1203     1204       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

ben-grande · 2025-06-03T16:36:27Z

PipelineRetryFailed

marmarta · 2025-06-10T15:26:38Z

+        if is_linux and self.init_mem.value() * 10 < self.max_mem_size.value():
            self.warn_too_much_mem_label.setVisible(True)

+    def check_disp_changes(self):


what does this function actually do?

Nothing, outdated, removed.

ben-grande · 2025-06-17T18:44:37Z

Failing test is unrelated to this PR and related to qubesimgconverter.

For: QubesOS/qubes-issues#1512

ben-grande · 2025-06-18T02:51:04Z

PipelineRetryFailed

ben-grande force-pushed the preload-dispvm branch 6 times, most recently from 8a9c8b1 to 9369545 Compare May 22, 2025 14:38

ben-grande changed the title ~~[WIP] Preload dispvm~~ Add disposable preload setting to advanced settings May 22, 2025

ben-grande mentioned this pull request May 22, 2025

Preloaded disposables QubesOS/qubes-issues#1512

Open

32 tasks

ben-grande force-pushed the preload-dispvm branch 2 times, most recently from c639372 to 315926b Compare May 22, 2025 16:07

marmarta suggested changes May 23, 2025

View reviewed changes

ben-grande force-pushed the preload-dispvm branch 3 times, most recently from 942f71a to ab68aa1 Compare May 23, 2025 17:10

marmarek added openqa-pending and removed openqa-pending labels May 25, 2025

ben-grande force-pushed the preload-dispvm branch from ab68aa1 to b580cc1 Compare June 2, 2025 15:10

Black formatter

31c9d53

ben-grande force-pushed the preload-dispvm branch 5 times, most recently from 4087322 to 2680b07 Compare June 3, 2025 11:43

ben-grande force-pushed the preload-dispvm branch 2 times, most recently from a555607 to 207745b Compare June 3, 2025 16:32

marmarek added the openqa-pending label Jun 5, 2025

marmarta suggested changes Jun 10, 2025

View reviewed changes

ben-grande force-pushed the preload-dispvm branch from 207745b to 051b7f6 Compare June 10, 2025 16:26

qubesos-bot mentioned this pull request Jun 11, 2025

Merge #258 and #262 QubesOS/qubes-desktop-linux-manager#266

Closed

ben-grande force-pushed the preload-dispvm branch 2 times, most recently from 44147ca to e866884 Compare June 11, 2025 15:42

qubesos-bot mentioned this pull request Jun 12, 2025

Remove '$env' after it's loaded into environment QubesOS/qubes-gui-agent-linux#234

Merged

Add preload disposable option to advanced settings

16f7bd0

For: QubesOS/qubes-issues#1512

ben-grande force-pushed the preload-dispvm branch from e866884 to 16f7bd0 Compare June 17, 2025 19:05

marmarta approved these changes Jun 18, 2025

View reviewed changes

marmarek added openqa-group-1 and removed openqa-pending openqa-group-1 labels Jun 26, 2025

marmarek merged commit 0b96f01 into QubesOS:main Jun 26, 2025
4 of 5 checks passed

Uh oh!

Conversation

ben-grande commented May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

marmarta left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ben-grande commented May 23, 2025

Uh oh!

ben-grande commented May 23, 2025

Uh oh!

codecov Bot commented Jun 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ben-grande commented Jun 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ben-grande commented Jun 17, 2025

Uh oh!

ben-grande commented Jun 18, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ben-grande commented May 22, 2025 •

edited

Loading

codecov Bot commented Jun 3, 2025 •

edited

Loading