Discard preloaded disposables with outdated props

[ben-grande]

If a qube property is changed on the disposable template, it is not replicated to the preloaded disposable, no refresh occurs, which means, if a disposable template has the netvm changed, the preloaded disposable would still remain with the old setting. While changing netvm on the fly is possible after unpause, several other settings requires a restart.

Fixes: QubesOS/qubes-issues#10525
For: QubesOS/qubes-issues#1512

---

I think checking features is not relevant: QubesOS/qubes-core-admin-client#398 (comment)

[codecov]

Codecov Report

❌ Patch coverage is 59.52381% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.17%. Comparing base (e23e32d) to head (f03fe7b).
⚠️ Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
qubes/vm/mix/dvmtemplate.py	52.38%	10 Missing ⚠️
qubes/vm/dispvm.py	73.68%	5 Missing ⚠️
qubes/vm/mix/net.py	0.00%	1 Missing ⚠️
qubes/vm/templatevm.py	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #771      +/-   ##
==========================================
- Coverage   70.21%   70.17%   -0.05%     
==========================================
  Files          61       61              
  Lines       13942    13975      +33     
==========================================
+ Hits         9790     9807      +17     
- Misses       4152     4168      +16     

Flag	Coverage Δ
unittests	70.17% <59.52%> (-0.05%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

[qubesos-bot]

OpenQA test summary

Complete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=202601122238-4.3&flavor=pull-requests

Test run included the following:

• Discard preloaded disposables with outdated props #771

New failures, excluding unstable

Compared to: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2026011214-devel&flavor=update

Failed tests

No failures!

Fixed failures

Compared to: https://openqa.qubes-os.org/tests/165203#dependencies

4 fixed

• system_tests_dispvm

  • system_tests: Fail (unknown)
    Tests qubes.tests.integ.dispvm failed (exit code 1), details report...

  • system_tests: Failed (test died)
    # Test died: Some tests failed at qubesos/tests/system_tests.pm lin...

  • TC_20_DispVM_whonix-workstation-18: test_015_preload_race_more (error)
    raise TimeoutError from exc_val... TimeoutError

  • TC_20_DispVM_whonix-workstation-18: test_016_preload_race_less (error)
    raise TimeoutError from exc_val... TimeoutError

Unstable tests

Details

Performance Tests

Performance degradation:

No issues

Remaining performance tests:

No remaining performance tests

[marmarek]

Shouldn't netvm be excluded too, given you added handling for that already? Or is that handling not complete? There are (at least) two cases:

• disposable template uses default netvm, and the global default changes
• disposable template uses non-default netvm (none or not), and user changes this value

If I read the code correctly, currently only the first case is handled by the deferred netvm setting, right?
But if going with discarding preloaded disposables on netvm change too, then maybe deferred netvm change can be removed? At least its use in preloaded disposables code?

[ben-grande]

Shouldn't netvm be excluded too, given you added handling for that already? Or is that handling not complete?

Handling not complete, examples below.

There are (at least) two cases:

* disposable template uses default netvm, and the global default changes

* disposable template uses non-default netvm (none or not), and user changes this value

If I read the code correctly, currently only the first case is handled by the deferred netvm setting, right?

Right. The current PR would allow handling the second case. Note that deferred netvm handles reestablishing severed link in case the netvm restarts (domain-shutdown in vm.mix.net), while the current PR wouldn't know about it as netvm property would not have changed.

But if going with discarding preloaded disposables on netvm change too, then maybe deferred netvm change can be removed? At least its use in preloaded disposables code?

Here are some cases. It always involves a paused client and severed link. I will consider changes only for vm switch (choosing a different netvm), not for state change (restart):

• preloaded disposable netvm restarts: no property change, handled by deferred netvm
• disposable template netvm changes: property changed, handled by this PR

The deferred netvm PR introduced the feature deferred-netvm-original, which could be placeholders for preloaded disposables to discard early? Now instead of trying to establish netvm link after unpause, it will establish on a new preloaded qube, but without the speed benefits of preloading.

The behavior them will change:

• Changing default_netvm will discard if disposable netvm is the default
• Change disposable template netvm will restart
• Restarting netvm will refresh

If you update via the Dom0 updater, it will shutdown the template (refreshes preloaded disposables because of outdated volumes, already the case) and maybe restart servicevms that are netvms (discards disposables because of severed link, new). So behavior after update is, no preloaded disposables. There are some ways to solve this, and I'd like to solve tool independent, so it doesn't matter if you use Salt,Ansible or the updater:

• Refresh with a generous delay, so when changing properties, it doesn't try to refresh multiple times in a short amount of time.

[ben-grande]

The current discard code is lazily evaluated, only when requesting a disposable, which means it won't have any preload ready. It could also be improved by having a wildcard handler on qubes/vm/mix/dvmtemplate.py:

@qubes.events.handler("property-reset:*")
@qubes.events.handler("property-set:*")

Outdated volumes is already handled on domain-shutdown, I guess it makes sense to anticipate discarding earlier, maybe refresh, will see.

[marmarek]

IMO better do it lazily, to better handle several changed properties.

[ben-grande]

IMO better do it lazily, to better handle several changed properties.

Outdated volumes is done on template shutdown and lazily if somehow the domain-shutdown was not called yet.

I could add a 10-15s delay till refresh (or even larger delay)? Or not good enough?

[ben-grande]

I added some delay and pushed to test the behavior. If the property-(re)\?set code is unwanted, I will just remove one block.

[ben-grande]

Increasing private storage of the disposable template doesn't reflect in vol.is_outdated(), searching for another method.

Hm, this feels like a bug in is_outdated() method, intuitively outdated size should also be considered outdated. But then, there is a case of resizing root volume of a template-based qube (it's a valid operation, it won't persist after restart, but still one can do it to have temporarily more space in rootfs) and that IMO shouldn't be considered outdated. And similar for changing root/private volume size of a disposable. So, simple size check is not good enough for generic vol.is_outdated() case...

But, for preloaded case, size check might be enough.

Seems to be enough to check itself against the disposable template volume_config["private"]["size"]?

[marmarek]

Seems to be enough to check itself against the disposable template volume_config["private"]["size"]?

Yes, for preloaded case should be enough.

[ben-grande]

PipelineRetryFailed

[marmarek]

openQArun TEST=system_tests_dispvm

[ben-grande]

Interesting that reading the logs doesn't show it doesn't discard the preloads on all runs, only on the runs that it was supposed to (test_0{19,20}):

• https://openqa.qubes-os.org/tests/165251/file/system_tests-tests-qubes.tests.integ.dispvm.log

Because if it did, it would error out as there are assertions to make sure it received the expected qube from the list:

qubes-core-admin/qubes/tests/integ/dispvm.py

Line 472 in 756ff94

self.assertEqual(stdout, dispvm_name)

It is some behavior seem after qubesd restarts.