On 03/28/2013 01:16 PM, Reshetova, Elena wrote:
On 03/27/2013 02:34 PM, Reshetova, Elena wrote:
After far too much pondering... I went ahead and added the prepare
hook
+ some related bits and pieces. And actually ripped out SELinux
+ support
>from rpm core while at it, replaced by a simple SELinux plugin. Wohoo.
Looks cool :) Hope it works ;)
The basics seem to be working fine :)
The plugin configuration mechanism probably wants a bit more thinking + work
though: for some things you'd want to be able to enable a plugin by merely
installing the relevant (sub)package. For example I would want the SELinux
plugin to get enabled whenever its present, without having to hunt where
__transaction_plugins is defined and override it, which is annoying and
error-prone.
In other words, I think there should be a drop-in directory for the plugin
configuration where the plugin sub-packages can drop their default
configuration as separate files, including whether they should be enabled by
default or not. There was something else in this direction too ... but I
can't remember it >right now.
Yes, this is what I was wondering also. In our case I have the msm plugin in a
separate rpm binary and our build engineers would like to have an easy way to
switch it on and off, when building the image. And the most easiest way for
them is to just include/not include the rpm plugin package. So, I think having
this configurable separately would for sure make them happy and as you said
probably would be much more robust than defining this in macros.in file.
Right, agreed then.
I think I'll leave the commit-hooks to you though :)
Ok, I think I will be able to send you a version for review today, but
I have got one question. I was under impression that we at some point
agreed to pass to hooks the whole stat structure as opposite for just
mode_t. This would allow plugins to make checks on things like
st_nlink and other useful info about the file. Do I remember this wrongly?
No, you are right, but I chickened out :) See
http://rpm.org/gitweb?p=rpm.git;a=commitdiff;h=354c00ba7558e2dd78dd6f5906d3ba3e4c41e74a
http://rpm.org/gitweb?p=rpm.git;a=commitdiff;h=3ae4e4087e48e0bbfbafe5c9948bdcbc5fe1c63e
The trouble starts with the special case of fsmMkdirs(): there's a struct
stat handy, but the directories only get created if the stat() fails, in
which case the struct stat contents are undefined. Sure, it'd be possible to
hack up a struct stat that resembles a directory for that case, but that rang
a "proceed with >caution" alarm bell in my head. If, or rather when, we need
to fake up stuff the semantics get fuzzy real quick. For example the st_nlink
thing: the adjusted count is currently actually only available in fsmCommit()
so different hooks would see different values for the same file. Etc.
I still actually think we'll eventually want to pass the whole stat struct
(or roughly equivalent amount of data in some other means) to plugins, being
able to sanely do so requires further hacking of the FSM.
Should we do the stat passing then for fsmCommit hooks? I am attaching the
current state of my fsmCommit hooks just to show the place where I was
thinking to add them. I haven't checked the tabs and other stuff, so this is
just to give idea. I was thinking that instead of passing mode_t to the both
hooks (in this patch it still does that), the whole stat stuct can be passed
and this would give at least these hooks enough info. What do you think? Or if
is too bad/assymetric to have mode_t in other hooks and stat here?
I think we better leave the stat struct out of the picture for now. Its
not just that the information in the stat struct would be more than a
bit fuzzy, but also its not really sufficient either. For example a
%config management plugin would need to know whether a file is a %config
or not, and that's currently not available to plugins without jumping
through a lot of hoops. What we'd really need is being able to pass
rpmfi objects to the hooks (in addition to some other things), but that
requires largish changes in the rpm internals... But I've talked about
that long enough maybe its time to actually do it. It shouldn't be a
particularly *difficult* change, just fairly large and tedious one.
As for the preliminary patch, yes that's what I was thinking of as well
- in particular, calling the pre-commit hook before fsmBackup() so in
case a backup is needed, the pre-commit hook can grab the contents
before its moved out of the way. There are some open questions here too
however: if there's a failure, the post-commit hook doesn't really know
whether it was the backup that failed or the actual commit. And then
there's the special case of directory replacing something else, in which
case the backup is (and needs to be) taken in rpmPackageFilesInstall()
already. And then there's the whole erasure business to deal with...
Just wondering if there actually should be *yet another* hook for
backups, which would allow avoiding the ambiguity in fsmCommit() and
make plugins aware of all the scenarios in which backups are taken. So
many hooks, sigh... dunno. At least we're not in danger of running out
of "supported hooks" bits anymore :) I changed the plugin initialization
+ hook-calling system fairly radically over the weekend as discussed.
- Panu -
_______________________________________________
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
