-
Notifications
You must be signed in to change notification settings - Fork 145
Basic CLI implementation that wraps Virtualization.Framework #1
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
Conversation
fkorotkov
left a comment
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.
I wonder if there is a need to store the Ram at all. VMs start super fast on my M1 Max so I wonder if we'll ever need the suspension mechanism at all and it seems it was causing only issues in Parallels and Anka (specifically the time and network portability).
Sources/tart/VMConfig.swift
Outdated
| var version: Int = 0 | ||
| var ecid: VZMacMachineIdentifier | ||
| var hardwareModel: VZMacHardwareModel | ||
| var cpuCountMin: Int |
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.
Doesn't look like it's actually min. I was able to change it in the config from 2 to 4 and back. Every time VM was picking up the proper CPU count. I think it should be just cpuCoun.
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.
See 8375e3c.
Sources/tart/VMConfig.swift
Outdated
| var ecid: VZMacMachineIdentifier | ||
| var hardwareModel: VZMacHardwareModel | ||
| var cpuCountMin: Int | ||
| var memorySizeMin: UInt64 |
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.
Same here. Plus I think it's better to store it in Mb and not bytes. Easier for humans. 😅
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.
See 8375e3c.
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.
As for the units, I believe the conversion should be implemented in the CLI itself, because we already have things like ecid and hardwareModel in the configuration file do not make it any easier for humans already.
Sources/tart/VMConfig.swift
Outdated
| } | ||
|
|
||
| func save(toURL: URL) throws { | ||
| try JSONEncoder().encode(self).write(to: toURL) |
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.
Let's save it in a pretty format:
encoder.outputFormatting = .prettyPrintedThere 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.
See 348d977.
Sources/tart/VMDirectory.swift
Outdated
| var configURL: URL { self.baseURL.appendingPathComponent("config") } | ||
| var diskURL: URL { self.baseURL.appendingPathComponent("disk") } | ||
| var nvramURL: URL { self.baseURL.appendingPathComponent("nvram") } |
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.
How about having config.json, disk.bin and ram.bin?
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.
See 6fc868e.
Are you talking about RAM size? Because we're not storing the RAM itself at the moment. |
To run this, create a
tart.entitlementsfile:Sign the
tartbinary: