-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
libshare cleanup, NFS path escaping #13165
Conversation
080c1db
to
01341d0
Compare
3813c30
to
8da8eda
Compare
During the porting process I've followed the (well, weak) style by adding ones that take a protocol list with Sum API changes are confined to libzfs.h and libshare.h. |
Oh, it seems i hastily misread the manual; there's just no way to escape share paths on FreeBSD :) That being said, I think we should still try (which is the case in the current code) since the errors make much more sense:
vs
and we're not gonna pollute other lines for shares with newlines in the path. |
1dab591
to
1d9bd17
Compare
aef1f78
to
4792e6b
Compare
This makes it so we don't leak a consistent 64 bytes anymore, makes the searches simpler and faster, removes /all allocations/ from the driver (quite trivially, since they were absolutely needless), and makes libshare thread-safe (except, maybe, linux/smb, but that only does pointer-width loads/stores so it's also mostly fine, except for leaking smb_shares) Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Ahelenia Ziemiańska <[email protected]> Closes openzfs#13165
Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Ahelenia Ziemiańska <[email protected]> Closes openzfs#13165
Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Ahelenia Ziemiańska <[email protected]> Closes openzfs#13165
With the additional benefit of removing all the _all() functions and treating a NULL list as "all" ‒ the remaining all function is for all /datasets/, which is consistent with the rest of the API Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Ahelenia Ziemiańska <[email protected]> Closes openzfs#13165
Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Ahelenia Ziemiańska <[email protected]> Closes openzfs#13165
Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Ahelenia Ziemiańska <[email protected]> Closes openzfs#13165
Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Ahelenia Ziemiańska <[email protected]> Closes openzfs#13165 Closes openzfs#13153
Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Ahelenia Ziemiańska <[email protected]> Closes openzfs#13165
Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Ahelenia Ziemiańska <[email protected]> Closes openzfs#13165
Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Ahelenia Ziemiańska <[email protected]> Closes openzfs#13165
Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Ahelenia Ziemiańska <[email protected]> Closes openzfs#13165 Closes openzfs#13324
Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Ahelenia Ziemiańska <[email protected]> Closes openzfs#13165
This renders it thread-safe Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Ahelenia Ziemiańska <[email protected]> Closes openzfs#13165
Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Ahelenia Ziemiańska <[email protected]> Closes openzfs#13165
Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Ahelenia Ziemiańska <[email protected]> Closes openzfs#13165
Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Ahelenia Ziemiańska <[email protected]> Closes openzfs#13165
This makes it so we don't leak a consistent 64 bytes anymore, makes the searches simpler and faster, removes /all allocations/ from the driver (quite trivially, since they were absolutely needless), and makes libshare thread-safe (except, maybe, linux/smb, but that only does pointer-width loads/stores so it's also mostly fine, except for leaking smb_shares) Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Ahelenia Ziemiańska <[email protected]> Closes openzfs#13165
Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Ahelenia Ziemiańska <[email protected]> Closes openzfs#13165
Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Ahelenia Ziemiańska <[email protected]> Closes openzfs#13165
With the additional benefit of removing all the _all() functions and treating a NULL list as "all" ‒ the remaining all function is for all /datasets/, which is consistent with the rest of the API Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Ahelenia Ziemiańska <[email protected]> Closes openzfs#13165
Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Ahelenia Ziemiańska <[email protected]> Closes openzfs#13165
Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Ahelenia Ziemiańska <[email protected]> Closes openzfs#13165
Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Ahelenia Ziemiańska <[email protected]> Closes openzfs#13165 Closes openzfs#13153
Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Ahelenia Ziemiańska <[email protected]> Closes openzfs#13165
Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Ahelenia Ziemiańska <[email protected]> Closes openzfs#13165
Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Ahelenia Ziemiańska <[email protected]> Closes openzfs#13165
Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Ahelenia Ziemiańska <[email protected]> Closes openzfs#13165 Closes openzfs#13324
Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Ahelenia Ziemiańska <[email protected]> Closes openzfs#13165
Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Ahelenia Ziemiańska <[email protected]> Closes openzfs#13165 Closes openzfs#13324
Motivation and Context
The way libshare handled dispatch was honestly so weird. Also, the libzfs protocol that handles every function as _nfs and _smb is kinda insane and won't scale so I replaced it.
Also, #13153 and #13324
Description
See individual commit messages.
Currently on top of #13259.How Has This Been Tested?
These were made:
From these:
And then consequently removed as the datasets were deleted.
They were also exported fine. Well, mostly. Turns out
exportfs
recursively parses backslash escapes. – reported upstream hereI had a different way for parsing:
but it's more fragile (but, hardly), slower, and worse than just doing it this way. If we just agree on the escaped charset and stick to it, it won't be a problem.
Types of changes
Checklist:
TODOyeah. got carried away a bit, tooSigned-off-by
.