Hey Feri,
quick update.
On 11/28/2017 4:15 PM, Fabio M. Di Nitto wrote:
Hi Feri,
I have been talking with Jan and Chrissie about this branch, pro and cons etc.
I think we are all in agreement that we like your move towards modules and we would like you to continue this way.
There are few bits and pieces of the code we need to fix tho before I can merge any change of this magnitude.
- load_compress_lib and load_crypto_lib should be able to stay inside compress.c and crypto.c. I don´t think there is a need to move them in common.c
this is done in the current module branch.
we need to define something like #define KNET_INTERNAL_CRYPTO_API_VER so that we can use that either to identify the path on disk where to load the modules (ex: pkglibdir/crypto/$VER/ or $VER/crypto) or checked at load time (or both). this should address point 4 below.
log_msg should not be exported in the public API. This is a no-no from me ;) Either pass the pointer to the function to the init of a given module, or let´s find another way to report errors back from the modules to main libknet.
this is done in the current module branch.
- You need to start adding your name as Author/Contributor around or I´ll stop accepting your patches ;) that includes signing-off your commits :-)
Cheers Fabio
On 11/28/2017 10:33 AM, Fabio M. Di Nitto wrote:
On 11/28/2017 10:00 AM, Ferenc Wágner wrote:
"Fabio M. Di Nitto" fdinitto@redhat.com writes:
let me stop you a minute here.
Hey Fabio,
Thanks for looking at this experiment proactively! It reached the point of viability right now, so let's chat about them if you have the time.
Yeps, I have got time :-)
(The patch set creates some rather obvious cleanup opportunities, but I guess we can leave them for later.)
what is the end goal of this work?
I tried to summarize it in 96a92847:
Our current practice of dlopening foreign shared libraries is problematic for several reasons: * not portable: modules and shared libraries can be different object types * dependency information is invisible (our canaries mostly solve this) * hardwiring SONAMES breaks on transitions (KNET_PKG_SONAME solves this) * symbol versioning information is lost (theoretically solvable) The preferred way out is generating dynamically loaded private modules from the main source, which then rely on the dynamic linker to load the external symbols as usual.For a longer version please refer to my mail at https://lists.debian.org/debian-mentors/2017/11/msg00200.html and the links included by Guillem Jover.
Most of those concerns appears to be coming from a packing perspective. I understand them, and some of them are valid.
We originally didn´t implement the modules model because it has several downsides for little benefit, based on the experience we had with corosync modules in the past.
I'd be interested to learn about these downsides, could you please provide some hints or keywords to search for?
I don´t think we have recorded them, but here is the rundown:
- technically speaking using modules is no different than dlopening a
shared library. In that respect, we are simply moving the problem somewhere else. I could agree (based on the threads above) that containing the problem within the same upstream is probably saner. 2) it enforces a strict internal API/ABI between main libknet and the plugins. Making changes to those very complex, specially during updates (see also point 4). Right now we want to keep those API/ABI free to change and expand. 3) it´s very difficult to debug modules due the symbol resolving mechanism (Honza in CC can provide more details). 4) runtime operations (updates) can be nasty. You could have application X that has loaded libknet 1.0 with modules 1.0 apt-get update.. get libknet 1.1 or whatever, new plugins, application tries to load a module and kaboom.
#1 is mostly political/perception really. I don´t feel arguing much about it, but the main point is that we are a very active upstream and I don´t see a big issue (for now) to keep tracking other libraries. #2 i can probably live with, but that would have to be a project/group call. #3 i am not happy about. specially given that the project is still in it´s "early" days. this specifically has proven very challenging in some environments. #4 can be solved by adding some kind of hashing/signing mechanism of the modules (aka load only modules that match the build).
I think it might be beneficial to have a chat about those major changes before you implement them :-) I *hate* for people to waste time on stuff that might not merge.
Absolutely right. The true extent of the changes is exaggerated by the diffs (really, it just moves some definitions before their usage and removes lots of boilerplate -- wasting the entirety of my previous canary and soname works), but I'm still proposing an architectural change. Those preprocessor tricks were neat; modules seem to be a better solution to this problem set. Now tell me why they aren't. :)
The patch size doesn´t scare me at all.
I am really concerned about point #2 and point #3 above. #4 can be technically solved.
As for the code I have seen so far, please find another way to pass log_msg down to the plugins. I am not going to export it to the world :-)
Fabio
Devel mailing list Devel@lists.kronosnet.org http://lists.kronosnet.org/mailman/listinfo/devel
Devel mailing list Devel@lists.kronosnet.org https://lists.kronosnet.org/mailman/listinfo/devel