Propagate vm-config.net-* features through NetVM chain#791
Propagate vm-config.net-* features through NetVM chain#791Codingisinmyblud wants to merge 1 commit into
Conversation
Codingisinmyblud
force-pushed
the
fix-mtu-propagation
branch
5 times, most recently
from
9cc12f8 to
8c60b09
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #791 +/- ##
=======================================
Coverage 70.15% 70.15%
=======================================
Files 61 61
Lines 13995 14028 +33
=======================================
+ Hits 9818 9842 +24
- Misses 4177 4186 +9
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:
|
|
See further discussion in QubesOS/qubes-issues#10758 |
Signed-off-by: Qasim Khawaja <dotboss1010@gmail.com>
Codingisinmyblud
force-pushed
the
fix-mtu-propagation
branch
from
8c60b09 to
7344bb8
Compare
|
Issue description is outdated. |
| def effective_mtu(self): | ||
| """Effective MTU for this domain. Inherited from netvm if not | ||
| set explicitly.""" | ||
| netvm = self |
There was a problem hiding this comment.
Please don't refer to a qube that may not be a netvm as such, it makes reading a bit confusing.
| # pylint: disable=unused-argument | ||
| eff_mtu = self.effective_mtu | ||
| if eff_mtu is not None: | ||
| self.untrusted_qdb.write("/vm-config/net-mtu", str(eff_mtu)) |
There was a problem hiding this comment.
I don't think it needs to be in the vm-config namespace anymore.
| def effective_mtu(self): | ||
| """Effective MTU for this domain. Inherited from netvm if not | ||
| set explicitly.""" | ||
| netvm = self | ||
| while netvm is not None: | ||
| if hasattr(netvm, "mtu") and netvm.mtu is not None: | ||
| return netvm.mtu | ||
| netvm = getattr(netvm, "netvm", None) | ||
| return None |
There was a problem hiding this comment.
Alternative would be to build it into mtu property default value itself, I think it would be easier this way. Something like:
def default_mtu(self):
if self.netvm:
return self.netvm.mtu
return Noneand then use that function as default= in the property definition.
| """Propagate mtu changes to connected VMs.""" | ||
|
|
||
| # pylint: disable=unused-argument | ||
| def _update_descendants(netvm): |
There was a problem hiding this comment.
Following the single mtu property pattern proposed above, this may fire property-reset:mtu event on VMs with vm.property_is_default("mtu") (instead of checking for not None`), and the rest be moved outside of the loop (to do it only for vm that got the event, not all its descendants - this would be handled by firing event there).
|
Hi. Do you plan to continue on this? |
|
Yeah, sorry just got a little busy. Should send a fix in a bit hopefully |
3 participants
This patch fixes the Qubes OS bug #10758, propagating vm-config.net-* features to the descendant qubes in the NetVM chain, thus making the vm-config.net-* features visible in the Qubes DB of the descendant qubes.
In the current implementation, if a NetVM has a vm-config.net-mtu, the descendant qubes will not inherit the vm-config.net-mtu automatically. However, the in-qube networking scripts of the descendant qubes read the configuration from the Qubes DB.
This patch introduces the propagation of the vm-config.net-* features from the NetVM chain to the descendant qubes. The propagation code has been introduced in the file qubes/vm/mix/net.py. The propagation code performs the following actions:
on domain-qdb-create: the qube inherits the effective vm-config.net-* features from the NetVM chain
on domain-feature-set:, the vm-config.net- features of the NetVM are propagated to the running descendant qubes, except when overridden
on domain-feature-delete:, the descendants of the NetVM inherit the next upstream vm-config.net- feature or delete the entry if there are no more upstream features
Fixes: QubesOS/qubes-issues#10758