-
Notifications
You must be signed in to change notification settings - Fork 327
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
Partial support for raw devices #76
Conversation
b185435
to
34d0d56
Compare
goto err; | ||
} | ||
assert(blocks != 0); | ||
assert(sectsz != 0); |
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.
These should probably be dynamic checks that also perror
, in case we compile with asserts disabled.
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.
Since they were commented out before I presume these originally come from bhyve and it would be better not to diverge on such trivial things. Maybe bhyve could be changed too though.
To that end, rebasing this PR onto #75 seems like a good idea to me.
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.
Agreed its not worth the divergence.
@@ -618,17 +618,17 @@ blockif_open(const char *optstr, const char *ident) | |||
sectsz = DEV_BSIZE; | |||
psectsz = psectoff = 0; | |||
candelete = geom = 0; | |||
blocks = 0; |
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.
Please indent to match the surrounding code (and again in the following if/ioctl combination).
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.
Sorry, editor fail :-(
Nice work. Apart from my comments on the code it would be good if the commit message incorporated the info from the PR text too since it is useful reference information IMHO. I expect |
34d0d56
to
6f1c31c
Compare
Regarding the |
LGTM but please rebase onto master now that #75 has been merged (should make your patch a lot smaller too, which is good!) |
1 similar comment
LGTM but please rebase onto master now that #75 has been merged (should make your patch a lot smaller too, which is good!) |
328640c
to
fdda0f7
Compare
Support for using virtual disk images. Example: ``` hdiutil create -megabytes 20 -fs "MS-DOS" disk hdiutil attach disk.dmg // Get disk number N xhyve ... -s 4:0,ahci-hd,/dev/rdiskN ... ``` Signed-off-by: Johannes Würbach <johannes.wuerbach@googlemail.com>
fdda0f7
to
3a1af93
Compare
Rebased 👍 |
Added a small test in a separate commit, wdyt? |
Test whether a disk image is mountable & writable inside the VM and whether the result is readable again by macOS. Signed-off-by: Johannes Würbach <johannes.wuerbach@googlemail.com>
390115b
to
025c34d
Compare
LGTM, merging, thanks |
I haven't checked, whether all kinds of volumes work, but virtual disk images do, which allows you to use sparse volumes now with xhyve.
Example:
Ported from machyve/xhyve#80