-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
WIP : Run as snapshot for macos "Apple Virtualisation" #3792
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, this is a great start!
Few notes (just from looking at the code):
- In order to not complicate it with saving and loading the snapshot bookmark, call
cleanupDriveSnapShot
when the VM stops. - Does it make sense that enabling snapshot is done in the Drive settings for each drive? The QEMU version currently uses adds an item to the context (right click) menu in the main screen.
- Looking more at the code, I don't really like this architecture. It doesn't seem right that
resetDriveSnapShot
is always called and internally it handles the logic based on whether each diskImage hasrunAsSnapshot
set. Maybe if the function was renamed to something more descriptive ("reset" doesn't really make sense for what it's doing) it would be okay. - Use either
snapshot
orSnapshot
in your function names, but notSnapShot
, since snapshot is one word. - In swift, spaces are not put on the inside of parentheses.
// Good:
foo(bar: baz)
// Bad:
foo( bar: baz )
@ktprograms - hmm i think the flag can be done on either a drive by drive level - or on an entire VM level. I was just generally following the QEMU API convention, where you can specify this on a drive level. The VM level option, is from what I understand, simply an alias to apply on all drives. While there are use cases (like having a /tmp drive always running as a snapshot). Or inversely, having a storage drive persist for specific configs. Not sure how much demand this is for this flow. Separately, doing so at the drive level makes it more obvious that the "--snapshot" does not apply to shared drive mounts. Either option fulfils my goal of a "disposable VM". For the rest, I will be applying your suggestions |
Also for my workflow ( from qubesOS ) - the disposable VM's is always reuse and rested for every run. Right-Click, to run as --snapshot, is a flow where I can accidentally misclick on. While not end of the world (as I would have a cloned VM), will be annoying when doing malware analysis. |
Maybe the QEMU Correct me if I'm wrong, though. |
For the drive options, it can be done with
It's terribly buried within the docs. But both options are available. Docs reference : https://manpages.debian.org/jessie/qemu-system-x86/qemu-system-x86_64.1.en.html |
Ah, I see. @osy what do you think about this? Should it be:
|
I vote for either of the last two =)
|
@ktprograms - applied the suggested changes, and added toggle for both disk level and system level. Note: because i can't build this on my laptop due to #3873 , please do a test build prior to merger >_<" I do wish there was an easier way for me to build this + validate on my end. |
You can download the sysroot for master from GitHub Actions, see the MacDevelopment document for instructions. Building the full sysroot is only necessary if you changed stuff with Qemu or the patches. |
Got it built, with some buggy behaviour with the current master branch (cant seem to boot an apple VM with just the master branch) - However, when i revert to v3.1.5 - i am able to get new apple VM's working again. Will be reapplying changes from v3.1.5 commit to avoid confusion with other changes (wip) On another note, i noticed in the config file there seems to be a pattern of using isX as a property. Should I be using runAsSnapshot, or isRunAsSnapshot ? |
I've switched to isX and hasX naming for bools to be consistent with apple libraries. So probably a good idea. |
Iterating on a few things, one thing i notice, for some reason the packaging command does not seem to be complete while being unsigned for the following command.
Last few lines of the command output
So from what i see, it builds the UTM.app in the tmp drive, but never moves it back to the build directory provided - not sure if the script is broken, or if the docs is outdated. (the built .app works to be clear) Anyway, been using the following as my general workaround / separate build script. And found it really useful when iterating on small changes. (in case anyone else finds it useful)
|
Update it now works - moving this to the new PR, as this was redone from a known "working commit" instead : #3893 |
#2688
The QEMU counterpart is
#3067
The general idea is to do a APFS clone (Meaning it will only require the delta additional space, and make no changes on the original) on VM startup. ie.
cp -c
command. This allows us to mimic aqemu <disk> --snapshot
like behaviour.However, while I have coded in C, i'm pretty much a swift noob, and is learning on the fly here. And would want feedback regarding this pull request. The code done here, has not been tested either, as I could not get it built on my machine, due to the lack of an apple developer account (unless i missed something else)
The goal is to support the disposable VM workflow (not so much on the snapshot management)