"Fabio M. Di Nitto" fdinitto@redhat.com writes:
On 11/29/2017 2:52 PM, Ferenc Wágner wrote:
"Fabio M. Di Nitto" fabbione@fabbione.net writes:
On 11/29/2017 8:50 AM, Fabio M. Di Nitto wrote:
On 11/28/2017 4:15 PM, Fabio M. Di Nitto wrote:
- 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.
For now I added an internal API version check. I am not sure yet we need to add also versioning on disk. It might be overkilling. What do you think?
I think module directory versioning would be better, as it allows coexistence of different module ABIs. That can help in your upgrade scenario, if the (already loaded and running) core library is upgraded and the modules with the old ABI version are left installed.
libknet1-1.0-x has internal API 1 libknet1-1.0-y has internal API 2
on update, the old dir would go away no matter what, unless you want to do metapackages similar to kernel and keep them around. IMHO, not worth it.
Consider the plugin packages as well:
libknet-1-crypto-nss ships /usr/lib/kronosnet-1/crypto_nss.so (ABI 1) libknet-2-crypto-nss ships /usr/lib/kronosnet-2/crypto_nss.so (ABI 2)
libknet1_x has internal ABI 1, recommends libknet-1-crypto-nss libknet1_y has internal ABI 2, recommends libknet-2-crypto-nss
Running application depends on libknet1, loads libknet.so.1.x into memory. Upgrade libknet1 to version y, it pulls in the new package libknet-2-crypto-nss. Running application can still load its ABI 1 NSS module, because libknet-1-crypto-nss is still installed. One restarted, it loads the new libknet.so.1.y, which can load the ABI 2 NSS module. It's all seamless from the user PoV if libknet-1-crypto-nss isn't removed too early.
The risky part is that libknet1_y may not pull libknet-2-crypto-nss strongly enough. Debian installs recommended packages by default, but not suggested ones. So suggested plugins may be lost during a minor upgrade of libknet1, which doesn't sound very kind... I haven't thought about metapackages yet.
All this complication does not arise if we just fail cleanly based on version checks. Which may be good enough for corosync, but hypothetic other cases may trigger module loading based on foreign network traffic, if I understood correctly. Anyway, I can't see problems adding directory versioning later if needed.
Speaking about the version check: is it necessary to store the version in the model description? It's a generic constant after all.
It is the easiest way to compare them and it´s only a uint8_t. Hardly a resource issue ;)
Certainly not, but it begs the question that when these ABI versions can be different. Actually, never. So why store it in each model instead of simply compiling it into the module loader routine?
I tested doing build with version 1, copy the modules, do a version 2 build, overwrite the modules, etc.. each module needs to know its own version.
Sure, it's compiled into each module as well. My point is that all models come from a single build of libknet.so, while the modules may come from different builds.
- 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.
Similarly to the above, is the function pointer worth storing in every handle? I understand it's handy, because it's being passed around anyway. My idea was writing it into another module symbol at load time and defining the logging macros to use that when compiling modules. Probably doesn't matter much, though.
I have tried to write it into a module symbol, but there was a remapping issue that had to be done per module. This one works across the board without per module changes. It´s a minor technicality IMHO.
You made me curious. Let me try to implement it... OK, I rebased and squashed my branch somewhat. I'm pushing it as modules2, we had some very similar ideas and some different ones!