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.

Sign up for GitHub

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

Jump to bottom

Refactor storage cache #57

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Refactor storage cache #57

wants to merge 2 commits into from

Conversation

germag
Copy link
Collaborator

@germag germag commented Jul 5, 2024

This is the first part of a larger refactor. We have some issues that need to be fixed:

  • Early on I made the mistake to store the disk images in the root of the cache directory, this will be fixed in the future.
  • The cache/storage logic is all over the place, this makes it difficult to access with the right locks. I can make this works without locks, just using atomic syscalls (i.e, rename, *xttr), but those are guaranteed to be atomic only on posix systems. AFAIK Windows doesn't provide atomic Fs operations (the use of TxF is discouraged).
  • We store all the metadata in a single json object, this also makes difficult to access it for different commands the just require partial info.
  • Modeling the Vm as an object and binding all the operation to that object (i.e, Run(), IsRunning(), Delete(), etc) cornered us making very difficult to add new features, like the BIB support, rebuilding the image if the disk size changes, etc.

I'm marking this PR as draft because depends on PR #56

@germag
REMOVE
aec89a6 
temporal commit, it will be included in another PR
@germag
Add a storage object
b4a8009 
This is the first part of a bigger refactoring. I tried to
make it as non-intrusive as possible, so it's not ideal.
But it will simplify the review, since this should have the
same behavior/mistakes than the previous version.

The current architecture of having all the actions as methods
of a VM object, and storing the config of the disk image, VM,
and the running Vm in s single json, is very inflexible.
In addition, the cache logic is all over the place, so let's
start with a simple cache implementation, that will allow us
to move to the final goal.

Signed-off-by: German Maglione <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@germag