-
Notifications
You must be signed in to change notification settings - Fork 149
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
Use common filesystem loader #1035
Conversation
Hi Brian,
I am generally OK with that though we should look to see if we can use the
SFS interface that teh xrootd protocol loads as opposed to loading it
twice. The bigger question is why are you using the SFS interface and not
the oss interface?
Andy
…On Wed, 7 Aug 2019, Brian P Bockelman wrote:
When the TPC library was maintained out-of-tree, we had to stick to the public interfaces. This forced the filesystem loader code to duplicate a lot of logic (not necessarily 100% correct).
With this change, the plugin will make a call directly into the internal function that does the SFS loading elsewhere in Xrootd.
@abh3 - if you are OK with the approach and implementation, there's another place where we can remove code in `XrdMacaroons`.
Fixes #1034
You can view, comment on, or merge this pull request online at:
#1035
-- Commit Summary --
* Fix const'ness of the loader declaration.
* Switch from homegrown plugin loader to the common one.
-- File Changes --
M src/XrdTpc/XrdTpcConfigure.cc (73)
M src/XrdXrootd/XrdXrootdConfig.cc (2)
M src/XrdXrootd/XrdXrootdLoadLib.cc (2)
-- Patch Links --
https://github.com/xrootd/xrootd/pull/1035.patch
https://github.com/xrootd/xrootd/pull/1035.diff
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#1035
|
I suspect we could - is it obvious how to "pull that through" to the plugin?
This code also needs to work on redirectors. |
Ah, I see. We could if we could gaurantee that xrootd would always be
loaded first. Unfortunately, people can subvert that by command line
overrides so it's not a sure thing. Mind you, you will potentially be
loading the SFS without any user specified wrappers.I assume you simply ar
ere looking for the "xrootd.fslib" directive to load the correct thing,
right?
Andy
…On Wed, 7 Aug 2019, Brian P Bockelman wrote:
> can [we] use the SFS interface that the xrootd protocol loads
I suspect we could - is it obvious how to "pull that through" to the plugin?
> The bigger question is why are you using the SFS interface and not the oss interface?
This code also needs to work on redirectors.
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#1035 (comment)
|
Correct! |
I see that this pull request is still in draft form and cannot be merged. Do you have an estimate of when this will be ready? |
Oh! I see that I missed Andy's last comment. Marking as ready-to-review. |
Not quite ready, as there seem to be conflicts now. In any case, we likely need to re-review this in the context of stack-able plugins that appears in R5. |
Can we close this pull request as it doesn't seem applicable after all the other changes around this issue. |
Can we close this pull request. It has merge conflicts and I think this was already fixed in another way, right? |
Is this pull request even relevant now? |
I think we can close this right now - can always come back as needed. |
When the TPC library was maintained out-of-tree, we had to stick to the public interfaces. This forced the filesystem loader code to duplicate a lot of logic (not necessarily 100% correct).
With this change, the plugin will make a call directly into the internal function that does the SFS loading elsewhere in Xrootd.
@abh3 - if you are OK with the approach and implementation, there's another place where we can remove code in
XrdMacaroons
.Fixes #1034