Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vboxwrapper: don't create projects/virtualbox #5658

Merged
merged 2 commits into from
Jul 19, 2024
Merged

Conversation

davidpanderson
Copy link
Contributor

If Vboxwrapper couldn't find the Vbox 'home dir' (normally ~/.VirtualBox) based on env vars, it would create one: projects/virtualbox (owned by boinc_projects, not boinc_master)

This is wrong because

  1. on startup, the client deletes anything in projects/
    that's not a project directory
  2. on Mac it causes installer error messages
    because the dir is not owned by boinc_master.

Fix: don't create projects/virtualbox.

If Vboxwrapper couldn't find the Vbox 'home dir' (normally ~/.VirtualBox)
based on env vars, it would create one: projects/virtualbox
(owned by boinc_projects, not boinc_master)

This is wrong because
1) on startup, the client deletes anything in projects/
    that's not a project directory
2) on Mac it causes installer error messages
    because the dir is not owned by boinc_master.

Fix: don't create projects/virtualbox.
@AenBleidd
Copy link
Member

@computezrmle, could you please review this change?

@@ -100,7 +100,9 @@ int VBOX_VM::initialize() {
}
#endif

// Determine the VirtualBox home directory. Overwrite as needed.
// Determine the VirtualBox home directory.
// NOTE: I'm not sure this is relevant; see
Copy link
Contributor

Choose a reason for hiding this comment

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

virtualbox_home_directory is used to locate VBoxSVC.log in vbox_common.cpp.
Hence, yes it needs to be set.

@computezrmle
Copy link
Contributor

The docs mention different directories to put VirtualBox.xml in depending on the host OS:

Linux and Oracle Solaris:$HOME/.config/VirtualBox.
Windows:$HOME/.VirtualBox.
macOS:$HOME/Library/VirtualBox.

It looks like the patch covers the situation on Windows only.

On Linux and Oracle Solaris XDG_CONFIG_HOME can be used instead of VBOX_USER_HOME.
This case may also be implemented.

@davidpanderson
Copy link
Contributor Author

Other than the comment, is the change OK?

Copy link

codecov bot commented Jul 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 10.53%. Comparing base (505b841) to head (0883a68).
Report is 46 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #5658      +/-   ##
============================================
- Coverage     10.53%   10.53%   -0.01%     
  Complexity     1068     1068              
============================================
  Files           279      279              
  Lines         35868    35874       +6     
  Branches       8409     8412       +3     
============================================
  Hits           3780     3780              
- Misses        31694    31700       +6     
  Partials        394      394              

see 3 files with indirect coverage changes

@davidpanderson davidpanderson merged commit 1e13fc8 into master Jul 19, 2024
146 checks passed
@davidpanderson davidpanderson deleted the dpa_vboxw_dir branch July 19, 2024 05:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Merged
Development

Successfully merging this pull request may close these issues.

None yet

3 participants