[Openais] [PATCH] trunk: objdb_reload implementation

[Fabio M. Di Nitto]

Hi Steven,

On Tue, 1 Jul 2008, Steven Dake wrote:

> Fabio
>
> Your patch looks fine but I'm going to hold off on commit until I have
> finished the objdb rework.  This may force some rework on your part
> (sorry).  Hoping today/tomorrowish - it is very close.
>

I found an interesting problem with the reload operation or generally the 
way in which this loop work (I have seen it in other bits of openais code 
as well).

static int object_reload_config(int flush, char **error_string)
{
        struct config_iface_ver0 **modules;
        int num_modules;
        int i;
        int res;

        main_get_config_modules(&modules, &num_modules);
        for (i=0; i<num_modules; i++) {
                if (modules[i]->config_reloadconfig) {
                        res = modules[i]->config_reloadconfig(&objdb_iface,
                                                      flush, error_string);
                        if (res)
                                return res;
                }
        }
        return 0;
}

So let's assume that in our scenario there is more than one plugin in 
charge of reload_config.

What happens is that we call:

plugingA->reloadconfig
if ok keep going
pluginB->reloadconfig
if ok keep going

Now.. what happens if pluingB fails? is pluginA supposed to rollback its 
changes? what if the 2 configs needs to be updated together?

My expectations as a user would be that the reload will fail, but would 
not trash the old config and that I would like to see some kind of config 
validator.

There are several ways to implement this, i will layout a couple of them, 
but i have no strong opinion on which one is best.

First:

within the same object_reload_config execute 2 loops:

        main_get_config_modules(&modules, &num_modules);
        for (i=0; i<num_modules; i++) {
                if (modules[i]->config_verifyconfig) {

and then execute:
        for (i=0; i<num_modules; i++) {
                if (modules[i]->config_reloadconfig) {

The config_verifyconfig call can either be public or private.. i see no 
problems either way, and it would sanity checks basically.

confdb_reload should never fail.

Second:

We implement verifyconfig as standalone, and the application will need to 
call confdb_verifyconfig before invoking _reloadconfig.

confdb_reload should never fail if config has been verified.

it would allow to have an external application to invoke a conf_verify as 
general tool to see if the config can be loaded.

Third (most expensive but best IMHO):

confdb_verifyconfig
backup objdb somewhere in mem
confdb_reloadconfig
on error, restore old objdb.

Fabio

--
I'm going to make him an offer he can't refuse.