qubes-core-agent-linux icon indicating copy to clipboard operation
qubes-core-agent-linux copied to clipboard

Misc-post-stop: remove unused QubesIncoming

Open kennethrrosen opened this issue 1 year ago • 5 comments

resolves QubesOS/qubes-issues#8307

kennethrrosen avatar Jul 22 '24 11:07 kennethrrosen

Can you please cleanup the git history here - include just the last commit?

marmarek avatar Jul 22 '24 11:07 marmarek

Can you please cleanup the git history here - include just the last commit?

@marmarek done

kennethrrosen avatar Jul 22 '24 13:07 kennethrrosen

I've made a minor change based on a discussion in the forum: https://forum.qubes-os.org/t/github-issue-8307-automatically-clean-up-qubesincoming-directory-if-empty/27798/14

kennethrrosen avatar Jul 22 '24 14:07 kennethrrosen

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 70.57%. Comparing base (0f346ad) to head (0bb7b9b). Report is 31 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #515   +/-   ##
=======================================
  Coverage   70.57%   70.57%           
=======================================
  Files           3        3           
  Lines         469      469           
=======================================
  Hits          331      331           
  Misses        138      138           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Jul 23 '24 15:07 codecov[bot]

Sorry for chiming in late. I had assumed that this functionality is intended to be optional (opt-in or opt-out) but it looks like it's always-on. Isn't the effect a bit intense with that in mind?

Deleting ~/QubesIncoming/source-vm/ if it's completely empty seems relatively uncontroversial to me. (Although the reasoning for not deleting an empty ~/QubesIncoming/ - vanishing file manager bookmarks - could apply here too for some people's workflows.)

But ~~recursively~~ deleting empty directories at arbitrary depths, and especially empty files, can "randomly" mangle complex directory trees. Like, occasionally I'll move a downloaded tarball or similar (e.g. a software package) from a DispVM to another VM and then leave all the extracted contents lying around for a while, right there inside of ~/QubesIncoming/source-vm/. These can contain meaningful empty directories or empty marker files.

rustybird avatar Aug 08 '24 17:08 rustybird

@marmarek resolved. gave this some time to let others chime in on the change.

kennethrrosen avatar Sep 29 '24 21:09 kennethrrosen

deleting empty directories at arbitrary depths, and especially empty files, can "randomly" mangle complex directory trees. Like, occasionally I'll move a downloaded tarball or similar (e.g. a software package) from a DispVM to another VM and then leave all the extracted contents lying around for a while, right there inside of ~/QubesIncoming/source-vm/. These can contain meaningful empty directories or empty marker files.

This is a valid concern. I'd say removing empty files/dirs directly in ~/QubesIncoming/source-vm/ (and empty source-vm itself) should be okay, but going deeper is problematic. And also, an opt-out mechanism would be useful, for example check for no-qubesincoming-cleanup qvm-service (skip if /run/qubes-service/no-qubesincoming-cleanup is present)

marmarek avatar Oct 04 '24 13:10 marmarek

and also, an opt-out mechanism would be useful, for example check for no-qubesincoming-cleanup qvm-service (skip if /run/qubes-service/no-qubesincoming-cleanup is present)

@marmarek I can add

# Check for opt-out mechanism
if [ -f /run/qubes-service/no-qubesincoming-cleanup ]; then
    exit 0
fi

And maybe even a note at the top of the script to instruct users on adding their own qubes service until it's added by default in the qube manager GUI? Or I could make a PR for that with some guidance from @alimirjamali

kennethrrosen avatar Oct 04 '24 14:10 kennethrrosen

There is a qvm-service discovery mechanism so it shows up in a dropdown in the qube settings (services tab). See qubes-rpc/post-install.d/10-qubes-core-agent-features.sh, you can add there qvm-features-request supported-service.no-qubesincoming-cleanup=1

marmarek avatar Oct 04 '24 14:10 marmarek

but going deeper is problematic

This is easily resolvable @kennethrrosen. You could add --maxdepth 1 to find commands to resolve this.

alimirjamali avatar Oct 04 '24 14:10 alimirjamali