vm: update only network QDB entries on netvm change#808
Conversation
|
/cc @ben-grande |
There was a problem hiding this comment.
Pull request overview
This PR changes how QubesDB entries are refreshed when a VM’s netvm changes while the VM is running, to avoid firing domain-qdb-create and rewriting unrelated QubesDB keys (notably /keyboard-layout), addressing QubesOS/qubes-issues#9892.
Changes:
- Add
QubesVM.update_qdb_netvm_entries()intended to update only network-related QubesDB keys. - Switch
NetVMMixin.on_property_set_netvm()fromcreate_qdb_entries()to the new network-only updater.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
qubes/vm/qubesvm.py |
Adds a new helper to update only network-related QubesDB entries on netvm change. |
qubes/vm/mix/net.py |
Uses the new helper during netvm changes for running VMs to avoid resetting unrelated QubesDB keys. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if self.visible_ip6: | ||
| self.untrusted_qdb.write("/qubes-ip6", str(self.visible_ip6)) | ||
| if self.visible_gateway6: | ||
| self.untrusted_qdb.write( | ||
| "/qubes-gateway6", str(self.visible_gateway6) | ||
| ) | ||
| else: | ||
| # netvm set to None — remove network entries | ||
| for key in ( | ||
| "/qubes-mac", | ||
| "/qubes-ip", | ||
| "/qubes-netmask", | ||
| "/qubes-gateway", | ||
| "/qubes-primary-dns", | ||
| "/qubes-secondary-dns", | ||
| "/qubes-ip6", | ||
| "/qubes-gateway6", | ||
| ): | ||
| self.untrusted_qdb.rm(key) |
marmarek
left a comment
marmarek
left a comment
There was a problem hiding this comment.
Besides copilot review (see my comments about it), this introduces code duplication. Adjust create_qdb_entries to call the new function in place of the copied code.
Done — |
|
Black complains about minor thing (see CI logs). Otherwise looks much better now. |
Done, fixed and squashed. |
Not really... note we use 80 chars line length.
This also didn't work, now here are 3 commits. Take a look here for example: https://stackoverflow.com/a/5189600 |
create_qdb_entries() fires domain-qdb-create which rewrites all QubesDB entries. Calling it on netvm change resets unrelated entries like /keyboard-layout, breaking user-customized keyboard layouts. Add update_qdb_netvm_entries() that updates only network-related entries and use it in on_property_set_netvm() instead of create_qdb_entries(). Fixes QubesOS/qubes-issues#9892 Signed-off-by: Nancy <9d.24.nancy.sangani@gmail.com> vm: update only network QDB entries on netvm change Calling create_qdb_entries() on netvm change fires domain-qdb-create which rewrites all QubesDB entries including /keyboard-layout, resetting any custom layout set by the user in the AppVM. Add update_qdb_netvm_entries() in QubesVM that writes only the network-related QubesDB entries (IP, gateway, netmask, DNS, MAC). Use it in create_qdb_entries() to avoid code duplication, and call it from on_property_set_netvm() instead of create_qdb_entries(). Update tests to mock update_qdb_netvm_entries() instead of create_qdb_entries() where netvm changes are tested. Fixes QubesOS/qubes-issues#9892 Signed-off-by: Nancy <9d.24.nancy.sangani@gmail.com> vm: update only network QDB entries on netvm change Calling create_qdb_entries() on netvm change fires domain-qdb-create which rewrites all QubesDB entries including /keyboard-layout, resetting any custom layout set by the user in the AppVM. Add update_qdb_netvm_entries() in QubesVM that writes only the network-related QubesDB entries (IP, gateway, netmask, DNS, MAC). Use it in create_qdb_entries() to avoid code duplication, and call it from on_property_set_netvm() instead of create_qdb_entries(). Update tests to mock update_qdb_netvm_entries() instead of create_qdb_entries() where netvm changes are tested. Fixes QubesOS/qubes-issues#9892 Signed-off-by: Nancy <9d.24.nancy.sangani@gmail.com> Format with black
nancysangani
force-pushed
the
fix-netvm-change-resets-keyboard-layout
branch
from
7b6a9f6 to
f5ac222
Compare
|
@marmarek PTAL when you get a chance. Thanks! |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #808 +/- ##
==========================================
+ Coverage 70.24% 70.40% +0.16%
==========================================
Files 61 61
Lines 14073 14110 +37
==========================================
+ Hits 9885 9934 +49
+ Misses 4188 4176 -12
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
The commit message is repeated 3 times... but the commit content itself is good now :) |
Fixes QubesOS/qubes-issues#9892
When netvm changes, on_property_set_netvm() called create_qdb_entries() which fires domain-qdb-create and rewrites all QubesDB entries including /keyboard-layout, resetting any custom layout set by the user.
Add update_qdb_netvm_entries() in QubesVM that writes only the network-related QubesDB entries (IP, gateway, netmask, DNS, MAC). Use this in on_property_set_netvm() instead of create_qdb_entries().