-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Update ZFS_IOC offsets for FreeBSD #9667
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.
Reviewed By: Allan Jude [email protected]
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.
Looks like you may have forgotten to include the matching ./tests/zfs-tests/cmd/libzfs_input_check/libzfs_input_check.c
changes.
3bedd5c
to
3008812
Compare
Fixed. |
include/sys/fs/zfs.h
Outdated
ZFS_IOC_FREEBSD = ZFS_IOC_FIRST + 0xc0, | ||
ZFS_IOC_NEXTBOOT, /* 0xc1 */ | ||
ZFS_IOC_JAIL, /* 0xc2 */ | ||
ZFS_IOC_UNJAIL, /* 0xc3 */ |
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.
Before we officially commit this range by using these reserved values let's check with @lundman. We may want to reserve smaller groups of 32 to leave space for MacOS and windows.
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.
Ok, on OSX:
+ 0xD0
and we define one (ZFS_IOC_PROXY_DATASET
)
On Windows:
+ 0xE0
and we define three (ZFS_IOC_MOUNT
. ZFS_IOC_UNMOUNT
, ZFS_IOC_UNREGISTER_FS
)
Unfortunately, Windows ioctls are a bit weird (they go up by increments of +4), so I had to spell them out:
https://github.com/openzfsonwindows/ZFSin/blob/master/ZFSin/zfs/include/sys/zfs_ioctl.h#L533
The CTL_CODE
macro shifts the "index" << 2
, hence the +4. So the Windows table might have to be separate, unless there is something clever we can do?
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.
(Hmm and I should probably fix that lone ZFS_IOC_RECV_NEW
o_O )
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.
Well these don't have to be entirely consistent across platforms, it more for our own convenience when reserving values. It sound like it may make the most sense for windows to do its own thing when we get to that point.
3008812
to
560ef72
Compare
include/sys/fs/zfs.h
Outdated
ZFS_IOC_FREEBSD = ZFS_IOC_FIRST + 0xc0, | ||
ZFS_IOC_NEXTBOOT, /* 0xc1 */ | ||
ZFS_IOC_JAIL, /* 0xc2 */ | ||
ZFS_IOC_UNJAIL, /* 0xc3 */ |
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.
Well these don't have to be entirely consistent across platforms, it more for our own convenience when reserving values. It sound like it may make the most sense for windows to do its own thing when we get to that point.
53051d8
to
b071969
Compare
Signed-off-by: Matt Macy <[email protected]>
b071969
to
45a0975
Compare
Codecov Report
@@ Coverage Diff @@
## master #9667 +/- ##
========================================
+ Coverage 79% 79% +<1%
========================================
Files 418 418
Lines 123562 123562
========================================
+ Hits 98093 98177 +84
+ Misses 25469 25385 -84
Continue to review full report at Codecov.
|
Signed-off-by: Matt Macy [email protected]
Motivation and Context
Description
How Has This Been Tested?
Types of changes
Checklist:
Signed-off-by
.