[Rpm-maint] FSM hooks for rpm plugin

[Panu Matilainen]

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 -