Skip to content

Allow named disposables in OpenInVM config#314

Open
Jayant-kernel wants to merge 1 commit into
QubesOS:mainfrom
Jayant-kernel:codex/openinvm-named-disposables
Open

Allow named disposables in OpenInVM config#314
Jayant-kernel wants to merge 1 commit into
QubesOS:mainfrom
Jayant-kernel:codex/openinvm-named-disposables

Conversation

@Jayant-kernel

Copy link
Copy Markdown

Fixes QubesOS/qubes-issues#9709.

What changed

  • Updated the Global Config Open in Disposable target filter to include named disposable qubes (DispVM with auto_cleanup disabled) in addition to disposable templates.
  • Kept volatile auto-cleanup disposable instances out of the selector.
  • Added widget tests covering named disposable inclusion and volatile disposable exclusion.

Validation

  • python -m py_compile qubes_config\global_config\rule_list_widgets.py qubes_config\tests\test_rule_list_widgets.py
  • git diff --check

python -m black ... and python -m pytest ... could not be run in this Windows checkout because the active Python environment does not have black or pytest installed.

@Jayant-kernel Jayant-kernel marked this pull request as ready for review May 29, 2026 12:21
@Jayant-kernel Jayant-kernel force-pushed the codex/openinvm-named-disposables branch from cbd2d3c to f8a4eed Compare May 29, 2026 12:21
@Jayant-kernel

Jayant-kernel commented May 29, 2026

Copy link
Copy Markdown
Author

@ben-grande
Look at the changes and is there any changes required ?

@ben-grande ben-grande left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR.

@staticmethod
def _dvm_template_filter(vm: qubesadmin.vm.QubesVM):
return getattr(vm, "template_for_dispvms", False)
def _dispvm_target_filter(vm: qubesadmin.vm.QubesVM):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A dvm is not the same as a dispvm. Maybe use _permanent_dispvm_and_dispvm_template_filter?

if getattr(vm, "template_for_dispvms", False):
return True

auto_cleanup = str(getattr(vm, "auto_cleanup", False)).lower() == "true"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The value can be compared as bool, no need to convert to str. Why was it compared as str?

rule = RuleDispVM(
Rule.from_line(
None,
"Service\t*\ttest-blue\t@dispvm\tallow\ttarget=@dispvm",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know that make_rule is using tabs and f-string with vars, but here it is str and the \t makes it hard to read. Use of \t is unnecessary.


assert not rule_row.target_widget.model.is_vm_available(
test_qapp.domains["test-disp"]
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests looks correct.

@ben-grande

Copy link
Copy Markdown

There is a new tab called "Disposables", at qubes_config/global_config/disposables.py. Some of the rows there should have the fix of this PR, but not all of them.

@Jayant-kernel Jayant-kernel force-pushed the codex/openinvm-named-disposables branch from f8a4eed to cc49496 Compare May 29, 2026 15:07
@Jayant-kernel

Copy link
Copy Markdown
Author

@ben-grande
I have used the proper dispvm filter, kept bool comparison, cleaned the test rule string, and applied the fix to the relevant Disposables rows.

@Jayant-kernel Jayant-kernel force-pushed the codex/openinvm-named-disposables branch 3 times, most recently from b2d2a1a to 638f161 Compare May 31, 2026 15:07
@codecov

codecov Bot commented May 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.70%. Comparing base (78bb7ba) to head (deb1882).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #314      +/-   ##
==========================================
- Coverage   92.72%   92.70%   -0.02%     
==========================================
  Files          66       66              
  Lines       13618    13640      +22     
==========================================
+ Hits        12627    12645      +18     
- Misses        991      995       +4     

☔ View full report in Codecov by Harness.
📢 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

Copy link
Copy Markdown

I have used the proper dispvm filter, kept bool comparison, cleaned the test rule string, and applied the fix to the relevant Disposables rows.

Skimmed through. Looks correct. Marta will review as she is responsible for this repo.

@marmarek marmarek requested a review from marmarta June 4, 2026 00:39
}
),
is_new_row=new,
filter_target=self.target_filter,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is minor, but can you use consistent naming? As in, make this "filter_target" everywhere (or rename the parameter to "target_filter", I'm good with either)

verb_description=verb_description,
initial_verb=_("will"),
filter_target=self._dvm_template_filter,
filter_target=(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like this approach. I don't want this function to be hard-coded here, please pass it to the constructor when needed and not hard-code it within this class.

@Jayant-kernel Jayant-kernel force-pushed the codex/openinvm-named-disposables branch from 638f161 to deb1882 Compare June 9, 2026 18:33
@Jayant-kernel

Copy link
Copy Markdown
Author

I made the naming consistent and moved the custom target filter out of DispvmRuleRow
is it okay

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Global Config: "Open in Disposable" should also list named disposables (in addition to disposable templates)

3 participants