Add handler for internal feature #55
Conversation
ben-grande
force-pushed
the
preload-dispvm
branch
from
21fd01b to
0f4d455
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #55 +/- ##
==========================================
- Coverage 81.44% 79.18% -2.26%
==========================================
Files 22 23 +1
Lines 2387 2561 +174
==========================================
+ Hits 1944 2028 +84
- Misses 443 533 +90 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ben-grande
force-pushed
the
preload-dispvm
branch
5 times, most recently
from
353aafa to
0dc860f
Compare
|
I'm a bit confused here: is the goal to hide every VM based on a template that's internal? Or do I misunderstand something? (also very possible) |
That is correct. If the qube is internal, hide it, but if it is not internal but it's template is, it also should be hidden. |
Hm, I'm a bit worried that this could be confusing... Could we document this in user-facing documentation somewhere? Preferably somewhere where the internal feature is described... |
|
I mean in the qubes-doc somewhere, too |
I grepped for And it doesn't even mention that it is a feature, just an internal qube. I think it would be wrong to add it to qubes-doct as only people deploying configuration for others do need to know about this feature and not end users. But if you are sure about this, please let me know in which page to add. |
|
Hmm, I see. But I keep thinking on it, and I think I figured out the problem case: if a system has been configured for end-user (like in SecureDrop case), you might get 'internal' templates managed by the system itself (auto-updated etc.) and qubes based on those templates that should very much be visible to the end-user. I think only qubes that are internal themselves should be hidden. |
|
A disposable template with internal feature set to true does not appear on
the menu.
If a child of this template has set to false the feature, the qube will
appear on the menu under another disposable template as if it was its child.
…On Tue, May 6, 2025, 4:20 PM Marta Marczykowska-Górecka < ***@***.***> wrote:
*marmarta* left a comment (QubesOS/qubes-desktop-linux-menu#55)
<#55 (comment)>
Hmm, I see. But I keep thinking on it, and I think I figured out the
problem case: if a system has been configured for end-user (like in
SecureDrop case), you might get 'internal' templates managed by the system
itself (auto-updated etc.) and qubes based on those templates that should
very much be visible to the end-user. I think only qubes that are internal
themselves should be hidden.
—
Reply to this email directly, view it on GitHub
<#55 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BCE2O4IAJTYHDCRSAZJR7HD25DALXAVCNFSM6AAAAAB3KNEILWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQNJUG42TSOJZGM>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
I'm confused. Currently yeah, I see that it is a problem, but I don't think a good solution is just not showing all qubes with template with internal (especially as I think this PR would also hide an AppVM whose template has 'internal' set to true - correct me if I'm wrong, if that's not true, then this reservation is partially withheld). I think this would have us think about how to show this case in the menu in a sensible way, not just hide it? |
A little bit of background. The preloaded disposables was setting the About a sensible way to show it, I don't know how. |
Hmm, so the issue is basically that an in-use preloaded dispvm shows up in the menu with a fake parent? My proposal would be then:
|
|
That is not an issue anymore on the preloaded side as mentioned above. It
has been fixed there by not deleting it if the disposable template has
internal set to true.
The preloaded qubes are not shown in the menu anymore because they also
have internal set to true.
This issue was in the beginning to account for the preloaded functionality,
but now it is more general.
…On Wed, May 7, 2025, 12:26 PM Marta Marczykowska-Górecka < ***@***.***> wrote:
*marmarta* left a comment (QubesOS/qubes-desktop-linux-menu#55)
<#55 (comment)>
A little bit of background. The preloaded disposables
<QubesOS/qubes-core-admin#660> was setting the
internal feature when preloading and emptying it when marking the
preloaded as used. Now, if the disposable template has the internal feature
set to true, it is not removing it anymore.
Hmm, so the issue is basically that an in-use preloaded dispvm shows up in
the menu with a fake parent? My proposal would be then:
- an initial bandaid: do not show only those VMs in the menu
(pre-loaded ones)
- next step: make an issue to rework how parent VM is shown to account
for VMs whose parent is internal (right now I'm torn between "show parent,
just don't allow running stuff in it" and "completely change how parents
are shown, aaaa"
—
Reply to this email directly, view it on GitHub
<#55 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BCE2O4MR4RWBNOS754AQUMD25HNU3AVCNFSM6AAAAAB3KNEILWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQNJYGAZDINRRGU>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
I think this looks fine on my side now, but codecov claims most of the actual internal-related lines are not covered by tests? |
ben-grande
force-pushed
the
preload-dispvm
branch
from
8e14ed8 to
8fa7ddf
Compare
From what I see, every failed check related to internal is a false positive. The changes pushed are because I changed my mind on querying the template features, using |
|
This is not resilient against not having access to the feature: It choked on dom0 specifically: I guess sys-gui should be able to get properties/features of dom0. But still, menu shouldn't explode if permissions are limited. |
ben-grande
force-pushed
the
preload-dispvm
branch
from
8fa7ddf to
2230243
Compare
Handling exception now. |
|
mypy complains, and looking at the message it's a valid complain |
ben-grande
force-pushed
the
preload-dispvm
branch
from
2230243 to
933d7d3
Compare
|
Can you split formatting and functional changes into separate commits? |
ben-grande
force-pushed
the
preload-dispvm
branch
from
933d7d3 to
5d9c320
Compare
|
PipelineRetry |
For: QubesOS/qubes-issues#1512
I didn't run the tests but I manually tested on R4.3 bench.
One thing I encountered is that a newly created qube can appear on the list for some miliseconds before it's internal feature is added.