----- Original Message -----
> From: "Omer Frenkel" <ofren...@redhat.com>
> To: "Eli Mesika" <emes...@redhat.com>
> Cc: "Dusmant Kumar Pati" <dp...@redhat.com>, "engine-devel"
> <engine-devel@ovirt.org>
> Sent: Sunday, December 1, 2013 11:02:46 AM
> Subject: Re: [Engine-devel] Using config values
>
>
>
> ----- Original Message -----
> > From: "Eli Mesika" <emes...@redhat.com>
> > To: "Dusmant Kumar Pati" <dp...@redhat.com>
> > Cc: "engine-devel" <engine-devel@ovirt.org>
> > Sent: Saturday, November 30, 2013 10:58:35 PM
> > Subject: Re: [Engine-devel] Using config values
> >
> >
> >
> > ----- Original Message -----
> > > From: "Dusmant Kumar Pati" <dp...@redhat.com>
> > > To: "Kanagaraj" <kmayi...@redhat.com>, "engine-devel"
> > > <engine-devel@ovirt.org>
> > > Sent: Friday, November 29, 2013 1:40:09 PM
> > > Subject: Re: [Engine-devel] Using config values
> > >
> > > On 11/29/2013 01:46 PM, Kanagaraj wrote:
> > >
> > >
> > > Hi All,
> > >
> > > The are some issues arising in configurations whenever we move up on the
> > > versions(3.3 => 3.4), because of the way we store and interpret them.
> > >
> > > Whenever there is a new cluster level, you will need to add a new entry
> > > for
> > > all(most) of the configuration. Mostly a copy paste if you see from 3.2
> > > to
> > > 3.3, except some CPU/PM type related configurations.
> > > Better option would be to have the defaul config value in
> > > ConfigValues.java
> > > and the overrides will go to config.sql. In this approach you don't need
> > > a
> > > new entries to config.sql when there is a new cluster level.
> > >
> > > Lets take an exmaple, "SupportForceCreateVG" - This is supported from 3.1
> > > onwards,
> > >
> > > If you look at config.sql, you will see following entries
> > > select fn_db_add_config_value('SupportForceCreateVG','false','3.0');
> > > select fn_db_add_config_value('SupportForceCreateVG','true','3.1');
> > > select fn_db_add_config_value('SupportForceCreateVG','true','3.2');
> > > select fn_db_add_config_value('SupportForceCreateVG','true','3.3');
> > >
> > > And in ConfigValues.java
> > >
> > > @TypeConverterAttribute(Boolean.class)
> > > @DefaultValueAttribute("false")
> > > SupportForceCreateVG,
> > >
> > > Now if there is 3.4 and 3.5, the user needs to add 2 more entries, which
> > > i
> > > feel is redundant.
> > >
> > > Instead we can make
> > >
> > > @TypeConverterAttribute(Boolean.class)
> > > @DefaultValueAttribute("true")
> > > SupportForceCreateVG,
> > >
> > > and have only the following in config.sql
> > > select fn_db_add_config_value('SupportForceCreateVG','false','3.0');
> > >
> > > if a particular value(for a specific cluster level) is not found in
> > > Config.sql, the fallback is to use the value available in
> > > ConfigValues.java.
> > >
> > > Please share your thoughts on this.
> >
> > Hi
> >
> > First of all I think its a good idea
> > I have 2 questions
> >
> > 1) Which value will be stored as default in the java class for
> > configuration
> > values that are not a boolean, that represents if a feature is active or
> > not.
> > Is that the latest version value ? sounds not obvious to me
> >
>
> i guess this will have to have a configuration values for each version in the
> db.
>
> > 2) There are some configuration values that are exposed to the user via the
> > engine-config tool, how this will work, we can not remove the entries their
> > since the user may change and override those values.
> >
>
> in your suggestion below, there is the same issue,
> if user want to change the 3.3 value, engine config will fail with "No such
> entry"
> because 3.3 will not be in the db..
> so if we are not fixing this, i think using the current implementation is
> good enough,
In this case engine-config will add this key with the given value for that
version, so , I see no problem in that...
In addition the getConfigValue will lookup for version matching only the first
time, so , if it was given a 3.4 version for key K and found a matching only
for 3.2 , it will add to the cache K with the same value for version 3.4
> no need to add logic, just to convert current options that are not in this
> format, to use this logic.
>
> > I have a different suggestion:
> > Default value will stay as is , meaning , it will reflect the value that
> > should be used to keep the application running correctly if a value is not
> > found in DB (which should not occur)
> >
> > Code of getting configuration value (getConfigValue(<key>,<version>) will
> > be
> > changed to get the closest version value to the given one.
> > For example , if a 3.4 version is given for a given <key> and we have in DB
> > just values for 3.0 and 3.1 , the 3.1 value is returned.
> > I prefer this solution since it makes the config.sql file self documented ,
> > showing only value changes and in which version each change occurred.
> >
> > To implement that, we should add this mechanism to the current code that
> > caches the DB content and as I see that it should be a simple change.
> > engine-config should be modified such that if the user change a value, this
> > value will be inserted to the database with the current release if not
> > exists and then the mechanism described above will get this value
> >
> > Example:
> >
> > VdsFenceType lists all the supported fencing agents for power management ,
> > it
> > currently has the following settings
> >
> > option_value
> > |
> > version
> > ---------------------------------------------------------------------------------------------+---------
> > alom,apc,bladecenter,drac5,eps,ilo,ilo3,ipmilan,rsa,rsb,wti,cisco_ucs
> > | 3.0
> > alom,apc,bladecenter,drac5,eps,ilo,ilo3,ipmilan,rsa,rsb,wti,cisco_ucs
> > | 3.1
> >
> > apc,apc_snmp,bladecenter,cisco_ucs,drac5,eps,ilo,ilo2,ilo3,ilo4,ipmilan,rsa,rsb,wti
> > | 3.2
> >
> > apc,apc_snmp,bladecenter,cisco_ucs,drac5,eps,ilo,ilo2,ilo3,ilo4,ipmilan,rsa,rsb,wti
> > | 3.3
> >
> > In the proposed solution, we will have
> >
> > option_value
> > |
> > version
> > ---------------------------------------------------------------------------------------------+---------
> > alom,apc,bladecenter,drac5,eps,ilo,ilo3,ipmilan,rsa,rsb,wti,cisco_ucs
> > | 3.0
> >
> > apc,apc_snmp,bladecenter,cisco_ucs,drac5,eps,ilo,ilo2,ilo3,ilo4,ipmilan,rsa,rsb,wti
> > | 3.2
> >
> > This is clear and documents only the changes done between versions and
> > serve
> > all values: boolean , string and complex type (those which requires any
> > kind
> > of parsing)
> >
> > What do you think?
> >
> > Eli
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > >
> > > Thanks,
> > > Kanagaraj
> > >
> > >
> > >
> > > _______________________________________________
> > > Engine-devel mailing list Engine-devel@ovirt.org
> > > http://lists.ovirt.org/mailman/listinfo/engine-devel
> > > I think, this is a good suggestion...
> > >
> > >
> > > _______________________________________________
> > > Engine-devel mailing list
> > > Engine-devel@ovirt.org
> > > http://lists.ovirt.org/mailman/listinfo/engine-devel
> > >
> > _______________________________________________
> > Engine-devel mailing list
> > Engine-devel@ovirt.org
> > http://lists.ovirt.org/mailman/listinfo/engine-devel
> >
>
_______________________________________________
Engine-devel mailing list
Engine-devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
