[Engine-devel] Using config values
This is a multi-part message in MIME format. --------------050008020702070900040001 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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. Thanks, Kanagaraj --------------050008020702070900040001 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit <html> <head> <meta http-equiv="content-type" content="text/html; charset=ISO-8859-1"> </head> <body bgcolor="#FFFFFF" text="#000000"> Hi All,<br> <br> 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.<br> <br> 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.<br> 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.<br> <br> Lets take an exmaple, "SupportForceCreateVG" - This is supported from 3.1 onwards,<br> <br> If you look at config.sql, you will see following entries <br> select fn_db_add_config_value('SupportForceCreateVG','false','3.0'); <br> select fn_db_add_config_value('SupportForceCreateVG','true','3.1'); <br> select fn_db_add_config_value('SupportForceCreateVG','true','3.2'); <br> select fn_db_add_config_value('SupportForceCreateVG','true','3.3');<br> <br> And in ConfigValues.java<br> <br> @TypeConverterAttribute(Boolean.class)<br> @DefaultValueAttribute("false")<br> SupportForceCreateVG,<br> <br> Now if there is 3.4 and 3.5, the user needs to add 2 more entries, which i feel is redundant.<br> <br> Instead we can make <br> <br> @TypeConverterAttribute(Boolean.class)<br> @DefaultValueAttribute("true")<br> SupportForceCreateVG,<br> <br> and have only the following in config.sql<br> select fn_db_add_config_value('SupportForceCreateVG','false','3.0');<br> <br> 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.<br> <br> Please share your thoughts on this.<br> <br> Thanks,<br> Kanagaraj<br> <br> <meta http-equiv="content-type" content="text/html; charset=ISO-8859-1"> </body> </html> --------------050008020702070900040001--
This is a multi-part message in MIME format. --------------060202090604020609010600 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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.
+1
Thanks, Kanagaraj
_______________________________________________ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
--------------060202090604020609010600 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit <html> <head> <meta content="text/html; charset=ISO-8859-1" http-equiv="Content-Type"> </head> <body bgcolor="#FFFFFF" text="#000000"> <br> <div class="moz-cite-prefix">On 11/29/2013 01:46 PM, Kanagaraj wrote:<br> </div> <blockquote cite="mid:52984D48.1070009@redhat.com" type="cite"> <meta http-equiv="content-type" content="text/html; charset=ISO-8859-1"> Hi All,<br> <br> 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.<br> <br> 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.<br> 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.<br> <br> Lets take an exmaple, "SupportForceCreateVG" - This is supported from 3.1 onwards,<br> <br> If you look at config.sql, you will see following entries <br> select fn_db_add_config_value('SupportForceCreateVG','false','3.0'); <br> select fn_db_add_config_value('SupportForceCreateVG','true','3.1'); <br> select fn_db_add_config_value('SupportForceCreateVG','true','3.2'); <br> select fn_db_add_config_value('SupportForceCreateVG','true','3.3');<br> <br> And in ConfigValues.java<br> <br> @TypeConverterAttribute(Boolean.class)<br> @DefaultValueAttribute("false")<br> SupportForceCreateVG,<br> <br> Now if there is 3.4 and 3.5, the user needs to add 2 more entries, which i feel is redundant.<br> <br> Instead we can make <br> <br> @TypeConverterAttribute(Boolean.class)<br> @DefaultValueAttribute("true")<br> SupportForceCreateVG,<br> <br> and have only the following in config.sql<br> select fn_db_add_config_value('SupportForceCreateVG','false','3.0');<br> <br> 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.<br> <br> Please share your thoughts on this.<br> </blockquote> <br> +1<br> <br> <blockquote cite="mid:52984D48.1070009@redhat.com" type="cite"> <br> Thanks,<br> Kanagaraj<br> <br> <meta http-equiv="content-type" content="text/html; charset=ISO-8859-1"> <br> <fieldset class="mimeAttachmentHeader"></fieldset> <br> <pre wrap="">_______________________________________________ Engine-devel mailing list <a class="moz-txt-link-abbreviated" href="mailto:Engine-devel@ovirt.org">Engine-devel@ovirt.org</a> <a class="moz-txt-link-freetext" href="http://lists.ovirt.org/mailman/listinfo/engine-devel">http://lists.ovirt.org/mailman/listinfo/engine-devel</a> </pre> </blockquote> <br> </body> </html> --------------060202090604020609010600--
This is a multi-part message in MIME format. --------------010309070708090904000001 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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.
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...
--------------010309070708090904000001 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit <html> <head> <meta content="text/html; charset=ISO-8859-1" http-equiv="Content-Type"> </head> <body bgcolor="#FFFFFF" text="#000000"> <div class="moz-cite-prefix">On 11/29/2013 01:46 PM, Kanagaraj wrote:<br> </div> <blockquote cite="mid:52984D48.1070009@redhat.com" type="cite"> <meta http-equiv="content-type" content="text/html; charset=ISO-8859-1"> Hi All,<br> <br> 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.<br> <br> 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.<br> 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.<br> <br> Lets take an exmaple, "SupportForceCreateVG" - This is supported from 3.1 onwards,<br> <br> If you look at config.sql, you will see following entries <br> select fn_db_add_config_value('SupportForceCreateVG','false','3.0'); <br> select fn_db_add_config_value('SupportForceCreateVG','true','3.1'); <br> select fn_db_add_config_value('SupportForceCreateVG','true','3.2'); <br> select fn_db_add_config_value('SupportForceCreateVG','true','3.3');<br> <br> And in ConfigValues.java<br> <br> @TypeConverterAttribute(Boolean.class)<br> @DefaultValueAttribute("false")<br> SupportForceCreateVG,<br> <br> Now if there is 3.4 and 3.5, the user needs to add 2 more entries, which i feel is redundant.<br> <br> Instead we can make <br> <br> @TypeConverterAttribute(Boolean.class)<br> @DefaultValueAttribute("true")<br> SupportForceCreateVG,<br> <br> and have only the following in config.sql<br> select fn_db_add_config_value('SupportForceCreateVG','false','3.0');<br> <br> 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.<br> <br> Please share your thoughts on this.<br> <br> Thanks,<br> Kanagaraj<br> <br> <meta http-equiv="content-type" content="text/html; charset=ISO-8859-1"> <br> <fieldset class="mimeAttachmentHeader"></fieldset> <br> <pre wrap="">_______________________________________________ Engine-devel mailing list <a class="moz-txt-link-abbreviated" href="mailto:Engine-devel@ovirt.org">Engine-devel@ovirt.org</a> <a class="moz-txt-link-freetext" href="http://lists.ovirt.org/mailman/listinfo/engine-devel">http://lists.ovirt.org/mailman/listinfo/engine-devel</a> </pre> </blockquote> I think, this is a good suggestion...<br> <br> </body> </html> --------------010309070708090904000001--
----- Original Message -----
From: "Dusmant Kumar Pati" <dpati@redhat.com> To: "Kanagaraj" <kmayilsa@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 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. 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
----- Original Message -----
From: "Eli Mesika" <emesika@redhat.com> To: "Dusmant Kumar Pati" <dpati@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" <dpati@redhat.com> To: "Kanagaraj" <kmayilsa@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, 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
----- Original Message -----
From: "Omer Frenkel" <ofrenkel@redhat.com> To: "Eli Mesika" <emesika@redhat.com> Cc: "Dusmant Kumar Pati" <dpati@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" <emesika@redhat.com> To: "Dusmant Kumar Pati" <dpati@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" <dpati@redhat.com> To: "Kanagaraj" <kmayilsa@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
On 12/01/2013 11:58 AM, Eli Mesika wrote:
----- Original Message -----
From: "Omer Frenkel" <ofrenkel@redhat.com> To: "Eli Mesika" <emesika@redhat.com> Cc: "Dusmant Kumar Pati" <dpati@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" <emesika@redhat.com> To: "Dusmant Kumar Pati" <dpati@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" <dpati@redhat.com> To: "Kanagaraj" <kmayilsa@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
I don't think 3.4 should inherit from 3.2, only from the default value. i do think the code knowing if it should ask by cluster or dc level, or at 'general' level is wrong, and asking by cluster level should fetch the 'general' level before going to code level default.
------=_Part_43195740_1618344988.1385878122945 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit ----- Original Message -----
Hi All, Hi Kanagaraj,
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. This has already been implemented, there are many "feature supported" configurations working like this (for example GlusterSupport).
I think a more interesting approach would be to move these out of the DB since these values don't really hav e a reson to be there. Since the entire thing is abstracted by "Gluster/FeatureSupported" classes then we can easily change mechanism (of course whatever code is not using it can be easily converted to use the mechanism) For example a simple enum could do the trick: ------------------------------------- EXAMPLE ------------------------------------- /** * Convenience class to check if a gluster feature is supported or not in any given version.<br> * Methods should be named by feature and accept version to check against. */ public class GlusterFeatureSupported { /** * @param version * Compatibility version to check for. * @return <code>true</code> if gluster support is enabled, <code>false</code> if it's not. */ public static boolean gluster(Version version) { return SupportedFeatures.GLUSTER.isSupportedOn(version); } /** * @param version * Compatibility version to check for. * @return <code>true</code> if gluster heavyweight refresh is enabled, <code>false</code> if it's not. */ public static boolean refreshHeavyWeight(Version version) { return SupportedFeatures.REFRESH_HEAVYWEIGHT.isSupportedOn(version); } /* More methods... */ enum SupportedFeatures { GLUSTER(Version.v3_0), REFRESH_HEAVYWEIGHT(Version.v3_0, Version.v3_1), /* More members */; private Set<Version> unsupportedVersions = new HashSet<Version>(); private SupportedFeatures(Version... versions) { unsupportedVersions.addAll(Arrays.asList(versions)); } public boolean isSupportedOn(Version version) { return !unsupportedVersions.contains(version); } } ------------------------------------- END EXAMPLE ------------------------------------- Thoughts? Regards, Mike
Please share your thoughts on this.
Thanks, Kanagaraj
------=_Part_43195740_1618344988.1385878122945 Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: quoted-printable <html><body><div style=3D"font-family: times new roman, new york, times, se= rif; font-size: 12pt; color: #000000"><hr id=3D"zwchr"><blockquote style=3D= "border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;= font-weight:normal;font-style:normal;text-decoration:none;font-family:Helve= tica,Arial,sans-serif;font-size:12pt;"> =20 =20 =20 =20 Hi All,</blockquote><div><br></div><div>Hi Kanagaraj,<br></div><div><br=
</div><blockquote style=3D"border-left:2px solid #1010FF;margin-left:5px;p= adding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decora= tion:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><br> The are some issues arising in configurations whenever we move up on the versions(3.3 =3D> 3.4), because of the way we store and interpret them.<br><br> 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.<br=
<br> 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.</blockquote><div><br></div><div>This has already bee= n implemented, there are many "feature supported" configurations working li= ke this (for example GlusterSupport).<br></div><div><br></div><div>I think = a more interesting approach would be to move these out of the DB since thes= e values don't really hav e a reson to be there.<br></div><div>Since the en= tire thing is abstracted by "Gluster/FeatureSupported" classes then we can = easily change mechanism (of course whatever code is not using it can be eas= ily converted to use the mechanism)<br></div><div><br></div><div>For exampl= e a simple enum could do the trick:<br></div><div>-------------------------= ------------ EXAMPLE -------------------------------------<br></div><div>/*= *<br> * Convenience class to check if a gluster feature is supported o= r not in any given version.<br><br> * Methods should be named by= feature and accept version to check against.<br> */<br>public class G= lusterFeatureSupported {</div><div> /**<br> &n= bsp; * @param version<br> * = Compatibility version to c= heck for.<br> * @return <code>true</code&g= t; if gluster support is enabled, <code>false</code> if it's no= t.<br> */<br> public static boole= an gluster(Version version) {<br> = return SupportedFeatures.GLUSTER.isSupportedOn(version);<br> &n= bsp; }<br><br> /**<br> * @param v= ersion<br> * &nb= sp; Compatibility version to check for.<br> &n= bsp; * @return <code>true</code> if gluster heavywe= ight refresh is enabled, <code>false</code> if it's not.<br>&nb= sp; */<br> public static boolean refres= hHeavyWeight(Version version) {<br> &nbs=
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.<br><br> Lets take an exmaple, "SupportForceCreateVG" - This is supported from 3.1 onwards,<br><br> If you look at config.sql, you will see following entries <br> select fn_db_add_config_value('SupportForceCreateVG','false','3.0'); <br> select fn_db_add_config_value('SupportForceCreateVG','true','3.1'); <br> select fn_db_add_config_value('SupportForceCreateVG','true','3.2'); <br> select fn_db_add_config_value('SupportForceCreateVG','true','3.3');<br>= <br> And in ConfigValues.java<br><br> @TypeConverterAttribute(Boolean.class)<br> @DefaultValueAttribute("false")<br> SupportForceCreateVG,<br><br> Now if there is 3.4 and 3.5, the user needs to add 2 more entries, which i feel is redundant.<br><br> Instead we can make <br><br> @TypeConverterAttribute(Boolean.class)<br> @DefaultValueAttribute("true")<br> SupportForceCreateVG,<br><br> and have only the following in config.sql<br> select fn_db_add_config_value('SupportForceCreateVG','false','3.0');<br= p; return SupportedFeatures.REFRESH_HEAVYWEIGHT.isSupportedOn(version);<br>= }<br></div><div><br></div><div> /* Mor= e methods... */</div><div><br> enum SupportedFeatures {<b= r> GLUSTER(Version.v3_0),<br>&nbs= p; REFRESH_HEAVYWEIGHT(Version.v3_0, Ve= rsion.v3_1),</div><div> /* More m= embers */;<br><br> private Set<= ;Version> unsupportedVersions =3D new HashSet<Version>();<br><br>&= nbsp; private SupportedFeatures(Version= ... versions) {<br> &n= bsp; unsupportedVersions.addAll(Arrays.asList(versions));<br> &n= bsp; }<br><br> &= nbsp; public boolean isSupportedOn(Version version) {<br> = return !unsupportedV= ersions.contains(version);<br> }<= br> }</div><div>------------------------------------- END= EXAMPLE -------------------------------------</div><div><br></div><div>Tho= ughts?<br></div><div><br></div><div>Regards,<br></div><div>Mike<br></div><b= lockquote style=3D"border-left:2px solid #1010FF;margin-left:5px;padding-le= ft:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none= ;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><br> Please share your thoughts on this.<br><br> Thanks,<br> Kanagaraj<br><br></blockquote></div></body></html> ------=_Part_43195740_1618344988.1385878122945--
------=_Part_21090047_1204116571.1385888119663
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
----- Original Message -----
> From: "Mike Kolesnik" <mkolesni@redhat.com>
> To: "Kanagaraj" <kmayilsa@redhat.com>
> Cc: "engine-devel" <engine-devel@ovirt.org>
> Sent: Sunday, December 1, 2013 8:08:42 AM
> Subject: Re: [Engine-devel] Using config values
> ----- Original Message -----
> > Hi All,
>
> Hi Kanagaraj,
> > 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.
>
> This has already been implemented, there are many "feature supported"
> configurations working like this (for example GlusterSupport).
> I think a more interesting approach would be to move these out of the DB
> since these values don't really hav e a reson to be there.
> Since the entire thing is abstracted by "Gluster/FeatureSupported" classes
> then we can easily change mechanism (of course whatever code is not using it
> can be easily converted to use the mechanism)
> For example a simple enum could do the trick:
> ------------------------------------- EXAMPLE
> -------------------------------------
> /**
> * Convenience class to check if a gluster feature is supported or not in any
> given version.<br>
> * Methods should be named by feature and accept version to check against.
> */
> public class GlusterFeatureSupported {
> /**
> * @param version
> * Compatibility version to check for.
> * @return <code>true</code> if gluster support is enabled, <code>false</code>
> if it's not.
> */
> public static boolean gluster(Version version) {
> return SupportedFeatures.GLUSTER.isSupportedOn(version);
> }
> /**
> * @param version
> * Compatibility version to check for.
> * @return <code>true</code> if gluster heavyweight refresh is enabled,
> <code>false</code> if it's not.
> */
> public static boolean refreshHeavyWeight(Version version) {
> return SupportedFeatures.REFRESH_HEAVYWEIGHT.isSupportedOn(version);
> }
> /* More methods... */
> enum SupportedFeatures {
> GLUSTER(Version.v3_0),
> REFRESH_HEAVYWEIGHT(Version.v3_0, Version.v3_1),
> /* More members */;
> private Set<Version> unsupportedVersions = new HashSet<Version>();
> private SupportedFeatures(Version... versions) {
> unsupportedVersions.addAll(Arrays.asList(versions));
> }
> public boolean isSupportedOn(Version version) {
> return !unsupportedVersions.contains(version);
> }
> }
> ------------------------------------- END EXAMPLE
> -------------------------------------
> Thoughts?
unless i didn't understand something, this is not good,
this should stay configurable by the users,
for example if some user experience some issues with a feature and want to turn it off/change the values..
(not all version configuration are boolean, some are different values to different versions, like cpu-list)
> Regards,
> Mike
> > Please share your thoughts on this.
>
> > Thanks,
>
> > Kanagaraj
>
> _______________________________________________
> Engine-devel mailing list
> Engine-devel@ovirt.org
> http://lists.ovirt.org/mailman/listinfo/engine-devel
------=_Part_21090047_1204116571.1385888119663
Content-Type: text/html; charset=utf-8
Content-Transfer-Encoding: quoted-printable
<html><body><div style=3D"font-family: times new roman, new york, times, se=
rif; font-size: 12pt; color: #000000"><div><br></div><div><br></div><hr id=
=3D"zwchr"><blockquote style=3D"border-left:2px solid #1010FF;margin-left:5=
px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-de=
coration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><b>Fr=
om: </b>"Mike Kolesnik" <mkolesni@redhat.com><br><b>To: </b>"Kanagara=
j" <kmayilsa@redhat.com><br><b>Cc: </b>"engine-devel" <engine-deve=
l@ovirt.org><br><b>Sent: </b>Sunday, December 1, 2013 8:08:42 AM<br><b>S=
ubject: </b>Re: [Engine-devel] Using config values<br><div><br></div><div s=
tyle=3D"font-family: times new roman, new york, times, serif; font-size: 12=
pt; color: #000000"><hr id=3D"zwchr"><blockquote style=3D"border-left:2px s=
olid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal=
;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-se=
rif;font-size:12pt;">
=20
=20
=20
=20
Hi All,</blockquote><div><br></div><div>Hi Kanagaraj,<br></div><div><br=
></div><blockquote style=3D"border-left:2px solid #1010FF;margin-left:5px;p=
adding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decora=
tion:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><br>
The are some issues arising in configurations whenever we move up on
the versions(3.3 =3D> 3.4), because of the way we store and
interpret them.<br><div><br></div>
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.<br=
>
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.<br><div><br></div>
Lets take an exmaple, "SupportForceCreateVG" - This is supported
from 3.1 onwards,<br><div><br></div>
If you look at config.sql, you will see following entries <br>
select fn_db_add_config_value('SupportForceCreateVG','false','3.0');
<br>
select fn_db_add_config_value('SupportForceCreateVG','true','3.1');
<br>
select fn_db_add_config_value('SupportForceCreateVG','true','3.2');
<br>
select fn_db_add_config_value('SupportForceCreateVG','true','3.3');<br>=
<div><br></div>
And in ConfigValues.java<br><div><br></div>
@TypeConverterAttribute(Boolean.class)<br>
@DefaultValueAttribute("false")<br>
SupportForceCreateVG,<br><div><br></div>
Now if there is 3.4 and 3.5, the user needs to add 2 more entries,
which i feel is redundant.<br><div><br></div>
Instead we can make <br><div><br></div>
@TypeConverterAttribute(Boolean.class)<br>
@DefaultValueAttribute("true")<br>
SupportForceCreateVG,<br><div><br></div>
and have only the following in config.sql<br>
select fn_db_add_config_value('SupportForceCreateVG','false','3.0');<br=
><div><br></div>
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.</blockquote><div><br></div><div>This has already bee=
n implemented, there are many "feature supported" configurations working li=
ke this (for example GlusterSupport).<br></div><div><br></div><div>I think =
a more interesting approach would be to move these out of the DB since thes=
e values don't really hav e a reson to be there.<br></div><div>Since the en=
tire thing is abstracted by "Gluster/FeatureSupported" classes then we can =
easily change mechanism (of course whatever code is not using it can be eas=
ily converted to use the mechanism)<br></div><div><br></div><div>For exampl=
e a simple enum could do the trick:<br></div><div>-------------------------=
------------ EXAMPLE -------------------------------------<br></div><div>/*=
*<br> * Convenience class to check if a gluster feature is supported o=
r not in any given version.<br><br> * Methods should be named by=
feature and accept version to check against.<br> */<br>public class G=
lusterFeatureSupported {</div><div> /**<br> &n=
bsp; * @param version<br> * =
Compatibility version to c=
heck for.<br> * @return <code>true</code&g=
t; if gluster support is enabled, <code>false</code> if it's no=
t.<br> */<br> public static boole=
an gluster(Version version) {<br> =
return SupportedFeatures.GLUSTER.isSupportedOn(version);<br> &n=
bsp; }<br><div><br></div> /**<br> =
* @param version<br> * &nbs=
p; Compatibility version to check for.<=
br> * @return <code>true</code> if glus=
ter heavyweight refresh is enabled, <code>false</code> if it's =
not.<br> */<br> public static boo=
lean refreshHeavyWeight(Version version) {<br> =
; return SupportedFeatures.REFRESH_HEAVYWEIGHT.isSupportedOn(ve=
rsion);<br> }<br></div><div><br></div><div> &n=
bsp; /* More methods... */</div><div><br> enum SupportedF=
eatures {<br> GLUSTER(Version.v3_=
0),<br> REFRESH_HEAVYWEIGHT(Versi=
on.v3_0, Version.v3_1),</div><div> =
; /* More members */;<br><div><br></div> =
; private Set<Version> unsupportedVersions =3D new HashSet<V=
ersion>();<br><div><br></div> =
private SupportedFeatures(Version... versions) {<br> =
; unsupportedVersions.addAll(Arra=
ys.asList(versions));<br> }<br><d=
iv><br></div> public boolean isSu=
pportedOn(Version version) {<br> &=
nbsp; return !unsupportedVersions.contains(version);<br>&=
nbsp; }<br> }</div><d=
iv>------------------------------------- END EXAMPLE ----------------------=
---------------</div><div><br></div><div>Thoughts?</div></div></blockquote>=
<div><br>unless i didn't understand something, this is not good,<br></div><=
div>this should stay configurable by the users,<br></div><div>for example i=
f some user experience some issues with a feature and want to turn it off/c=
hange the values.. </div><div>(not all version configuration are boolean, s=
ome are different values to different versions, like cpu-list)<br></div><bl=
ockquote style=3D"border-left:2px solid #1010FF;margin-left:5px;padding-lef=
t:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;=
font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><div style=3D"font-=
family: times new roman, new york, times, serif; font-size: 12pt; color: #0=
00000"><div><br></div><div><br></div><div>Regards,<br></div><div>Mike<br></=
div><blockquote style=3D"border-left:2px solid #1010FF;margin-left:5px;padd=
ing-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoratio=
n:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><br>
Please share your thoughts on this.<br><div><br></div>
Thanks,<br>
Kanagaraj<br><div><br></div></blockquote></div><br>____________________=
___________________________<br>Engine-devel mailing list<br>Engine-devel@ov=
irt.org<br>http://lists.ovirt.org/mailman/listinfo/engine-devel<br></blockq=
uote><div><br></div></div></body></html>
------=_Part_21090047_1204116571.1385888119663--
------=_Part_43206801_1748816137.1385888510809 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit ----- Original Message -----
----- Original Message -----
From: "Mike Kolesnik" <mkolesni@redhat.com>
To: "Kanagaraj" <kmayilsa@redhat.com>
Cc: "engine-devel" <engine-devel@ovirt.org>
Sent: Sunday, December 1, 2013 8:08:42 AM
Subject: Re: [Engine-devel] Using config values
----- Original Message -----
Hi All,
Hi Kanagaraj,
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.
This has already been implemented, there are many "feature supported" configurations working like this (for example GlusterSupport).
I think a more interesting approach would be to move these out of the DB since these values don't really hav e a reson to be there.
Since the entire thing is abstracted by "Gluster/FeatureSupported" classes then we can easily change mechanism (of course whatever code is not using it can be easily converted to use the mechanism)
For example a simple enum could do the trick:
------------------------------------- EXAMPLE -------------------------------------
/**
* Convenience class to check if a gluster feature is supported or not in any given version.<br>
* Methods should be named by feature and accept version to check against.
*/
public class GlusterFeatureSupported {
/**
* @param version
* Compatibility version to check for.
* @return <code>true</code> if gluster support is enabled, <code>false</code> if it's not.
*/
public static boolean gluster(Version version) {
return SupportedFeatures.GLUSTER.isSupportedOn(version);
}
/**
* @param version
* Compatibility version to check for.
* @return <code>true</code> if gluster heavyweight refresh is enabled, <code>false</code> if it's not.
*/
public static boolean refreshHeavyWeight(Version version) {
return SupportedFeatures.REFRESH_HEAVYWEIGHT.isSupportedOn(version);
}
/* More methods... */
enum SupportedFeatures {
GLUSTER(Version.v3_0),
REFRESH_HEAVYWEIGHT(Version.v3_0, Version.v3_1),
/* More members */;
private Set<Version> unsupportedVersions = new HashSet<Version>();
private SupportedFeatures(Version... versions) {
unsupportedVersions.addAll(Arrays.asList(versions));
}
public boolean isSupportedOn(Version version) {
return !unsupportedVersions.contains(version);
}
}
------------------------------------- END EXAMPLE -------------------------------------
Thoughts?
unless i didn't understand something, this is not good, this should stay configurable by the users, for example if some user experience some issues with a feature and want to turn it off/change the values.. (not all version configuration are boolean, some are different values to different versions, like cpu-list)
This is for API level compatibility. If VDSM doesn't support for example hot plug in 3.1 then the user can't just decide that it does and change it. Also, this is not changeable by user since it's not exposed by engine-config (nor should it be). This is strictly for the engine-VDSM API compatibility, not for other configs which are version specific.
Regards,
Mike
Please share your thoughts on this.
Thanks,
Kanagaraj
_______________________________________________
Engine-devel mailing list
Engine-devel@ovirt.org
------=_Part_43206801_1748816137.1385888510809 Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: quoted-printable <html><body><div style=3D"font-family: times new roman, new york, times, se= rif; font-size: 12pt; color: #000000"><hr id=3D"zwchr"><blockquote style=3D= "border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;= font-weight:normal;font-style:normal;text-decoration:none;font-family:Helve= tica,Arial,sans-serif;font-size:12pt;" data-mce-style=3D"border-left: 2px s= olid #1010FF; margin-left: 5px; padding-left: 5px; color: #000; font-weight= : normal; font-style: normal; text-decoration: none; font-family: Helvetica= ,Arial,sans-serif; font-size: 12pt;"><div style=3D"font-family: times new r= oman, new york, times, serif; font-size: 12pt; color: #000000" data-mce-sty= le=3D"font-family: times new roman, new york, times, serif; font-size: 12pt= ; color: #000000;"><div><br></div><div><br></div><hr id=3D"zwchr"><blockquo= te style=3D"border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;= color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-f= amily:Helvetica,Arial,sans-serif;font-size:12pt;" data-mce-style=3D"border-= left: 2px solid #1010FF; margin-left: 5px; padding-left: 5px; color: #000; = font-weight: normal; font-style: normal; text-decoration: none; font-family= : Helvetica,Arial,sans-serif; font-size: 12pt;"><b>From: </b>"Mike Kolesnik= " <mkolesni@redhat.com><br><b>To: </b>"Kanagaraj" <kmayilsa@redhat= .com><br><b>Cc: </b>"engine-devel" <engine-devel@ovirt.org><br><b>= Sent: </b>Sunday, December 1, 2013 8:08:42 AM<br><b>Subject: </b>Re: [Engin= e-devel] Using config values<br><div><br></div><div style=3D"font-family: t= imes new roman, new york, times, serif; font-size: 12pt; color: #000000" da= ta-mce-style=3D"font-family: times new roman, new york, times, serif; font-= size: 12pt; color: #000000;"><hr id=3D"zwchr"><blockquote style=3D"border-l= eft:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weig= ht:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Aria= l,sans-serif;font-size:12pt;" data-mce-style=3D"border-left: 2px solid #101= 0FF; margin-left: 5px; padding-left: 5px; color: #000; font-weight: normal;= font-style: normal; text-decoration: none; font-family: Helvetica,Arial,sa= ns-serif; font-size: 12pt;">Hi All,</blockquote><div><br></div><div>Hi Kana= garaj,<br></div><div><br></div><blockquote style=3D"border-left:2px solid #= 1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-= style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;fo= nt-size:12pt;" data-mce-style=3D"border-left: 2px solid #1010FF; margin-lef= t: 5px; padding-left: 5px; color: #000; font-weight: normal; font-style: no= rmal; text-decoration: none; font-family: Helvetica,Arial,sans-serif; font-= size: 12pt;"><br> The are some issues arising in configurations whenever we= move up on the versions(3.3 =3D> 3.4), because of the way we store and = interpret them.<br><div><br></div>Whenever there is a new cluster level, yo= u 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 con= figurations.<br> 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 lev= el.<br><div><br></div>Lets take an exmaple, "SupportForceCreateVG" - This i= s supported from 3.1 onwards,<br><div><br></div>If you look at config.sql, = you will see following entries <br> select fn_db_add_config_value('SupportF= orceCreateVG','false','3.0'); <br> select fn_db_add_config_value('SupportFo= rceCreateVG','true','3.1'); <br> select fn_db_add_config_value('SupportForc= eCreateVG','true','3.2'); <br> select fn_db_add_config_value('SupportForceC= reateVG','true','3.3');<br><div><br></div>And in ConfigValues.java<br><div>= <br></div> @TypeConverterAttribute(Boolean.class)<br> &nb= sp; @DefaultValueAttribute("false")<br> Supp= ortForceCreateVG,<br><div><br></div>Now if there is 3.4 and 3.5, the user n= eeds to add 2 more entries, which i feel is redundant.<br><div><br></div>In= stead we can make <br><div><br></div> @TypeConverterAttri= bute(Boolean.class)<br> @DefaultValueAttribute("true")<b= r> SupportForceCreateVG,<br><div><br></div>and have only= the following in config.sql<br> select fn_db_add_config_value('SupportForc= eCreateVG','false','3.0');<br><div><br></div>if a particular value(for a sp= ecific cluster level) is not found in Config.sql, the fallback is to use th= e value available in ConfigValues.java.</blockquote><div><br></div><div>Thi= s has already been implemented, there are many "feature supported" configur= ations working like this (for example GlusterSupport).<br></div><div><br></= div><div>I think a more interesting approach would be to move these out of = the DB since these values don't really hav e a reson to be there.<br></div>= <div>Since the entire thing is abstracted by "Gluster/FeatureSupported" cla= sses then we can easily change mechanism (of course whatever code is not us= ing it can be easily converted to use the mechanism)<br></div><div><br></di= v><div>For example a simple enum could do the trick:<br></div><div>--------= ----------------------------- EXAMPLE -------------------------------------= <br></div><div>/**<br> * Convenience class to check if a gluster featu= re is supported or not in any given version.<br><br> * Methods s= hould be named by feature and accept version to check against.<br> */<= br>public class GlusterFeatureSupported {</div><div> /**<= br> * @param version<br> *&= nbsp; Compatibi= lity version to check for.<br> * @return <code&g= t;true</code> if gluster support is enabled, <code>false</co= de> if it's not.<br> */<br> pu= blic static boolean gluster(Version version) {<br> &= nbsp; return SupportedFeatures.GLUSTER.isSupportedOn(version);<= br> }<br><div><br></div> /**<br> &= nbsp; * @param version<br> * = ; Compatibility versi= on to check for.<br> * @return <code>true<= /code> if gluster heavyweight refresh is enabled, <code>false</= code> if it's not.<br> */<br> = public static boolean refreshHeavyWeight(Version version) {<br> = return SupportedFeatures.REFRESH_HEAVYWEIGHT= .isSupportedOn(version);<br> }<br></div><div><br></div><d= iv> /* More methods... */</div><div><br> = ; enum SupportedFeatures {<br> GL= USTER(Version.v3_0),<br> REFRESH_= HEAVYWEIGHT(Version.v3_0, Version.v3_1),</div><div> = /* More members */;<br><div><br></div> = private Set<Version> unsupportedVersions =3D= new HashSet<Version>();<br><div><br></div> &n= bsp; private SupportedFeatures(Version... versions) {<br> = unsupportedVer= sions.addAll(Arrays.asList(versions));<br> &nb= sp; }<br><div><br></div> pu= blic boolean isSupportedOn(Version version) {<br> &n= bsp; return !unsupportedVersions.contai= ns(version);<br> }<br> = ; }</div><div>------------------------------------- END EXAMPLE -----= --------------------------------</div><div><br></div><div>Thoughts?</div></= div></blockquote><div><br>unless i didn't understand something, this is not= good,<br></div><div>this should stay configurable by the users,<br></div><= div>for example if some user experience some issues with a feature and want= to turn it off/change the values..</div><div>(not all version configuratio= n are boolean, some are different values to different versions, like cpu-li= st)</div></div></blockquote><div><br></div><div>This is for API level compa= tibility.<br></div><div>If VDSM doesn't support for example hot plug in 3.1= then the user can't just decide that it does and change it.<br></div><div>= Also, this is not changeable by user since it's not exposed by engine-confi= g (nor should it be).<br></div><div><br></div><div>This is strictly for the= engine-VDSM API compatibility, not for other configs which are version spe= cific.<br></div><div><br></div><blockquote style=3D"border-left:2px solid #= 1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-= style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;fo= nt-size:12pt;" data-mce-style=3D"border-left: 2px solid #1010FF; margin-lef= t: 5px; padding-left: 5px; color: #000; font-weight: normal; font-style: no= rmal; text-decoration: none; font-family: Helvetica,Arial,sans-serif; font-= size: 12pt;"><div style=3D"font-family: times new roman, new york, times, s= erif; font-size: 12pt; color: #000000" data-mce-style=3D"font-family: times= new roman, new york, times, serif; font-size: 12pt; color: #000000;"><div>= <br></div><blockquote style=3D"border-left:2px solid #1010FF;margin-left:5p= x;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-dec= oration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;" data-m= ce-style=3D"border-left: 2px solid #1010FF; margin-left: 5px; padding-left:= 5px; color: #000; font-weight: normal; font-style: normal; text-decoration= : none; font-family: Helvetica,Arial,sans-serif; font-size: 12pt;"><div sty= le=3D"font-family: times new roman, new york, times, serif; font-size: 12pt= ; color: #000000" data-mce-style=3D"font-family: times new roman, new york,= times, serif; font-size: 12pt; color: #000000;"><div><br></div><div><br></= div><div>Regards,<br></div><div>Mike<br></div><blockquote style=3D"border-l= eft:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weig= ht:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Aria= l,sans-serif;font-size:12pt;" data-mce-style=3D"border-left: 2px solid #101= 0FF; margin-left: 5px; padding-left: 5px; color: #000; font-weight: normal;= font-style: normal; text-decoration: none; font-family: Helvetica,Arial,sa= ns-serif; font-size: 12pt;"><br> Please share your thoughts on this.<br><di= v><br></div>Thanks,<br> Kanagaraj<br><div><br></div></blockquote></div><br>= _______________________________________________<br>Engine-devel mailing lis= t<br>Engine-devel@ovirt.org<br>http://lists.ovirt.org/mailman/listinfo/engi= ne-devel<br></blockquote><div><br></div></div></blockquote><div><br></div><= /div></body></html> ------=_Part_43206801_1748816137.1385888510809--
------=_Part_21094117_1742490232.1385888972528 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit ----- Original Message -----
From: "Mike Kolesnik" <mkolesni@redhat.com> To: "Omer Frenkel" <ofrenkel@redhat.com> Cc: "Kanagaraj" <kmayilsa@redhat.com>, "engine-devel" <engine-devel@ovirt.org> Sent: Sunday, December 1, 2013 11:01:50 AM Subject: Re: [Engine-devel] Using config values
----- Original Message -----
----- Original Message -----
From: "Mike Kolesnik" <mkolesni@redhat.com>
To: "Kanagaraj" <kmayilsa@redhat.com>
Cc: "engine-devel" <engine-devel@ovirt.org>
Sent: Sunday, December 1, 2013 8:08:42 AM
Subject: Re: [Engine-devel] Using config values
----- Original Message -----
Hi All,
Hi Kanagaraj,
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.
This has already been implemented, there are many "feature supported" configurations working like this (for example GlusterSupport).
I think a more interesting approach would be to move these out of the DB since these values don't really hav e a reson to be there.
Since the entire thing is abstracted by "Gluster/FeatureSupported" classes then we can easily change mechanism (of course whatever code is not using it can be easily converted to use the mechanism)
For example a simple enum could do the trick:
------------------------------------- EXAMPLE -------------------------------------
/**
* Convenience class to check if a gluster feature is supported or not in any given version.<br>
* Methods should be named by feature and accept version to check against.
*/
public class GlusterFeatureSupported {
/**
* @param version
* Compatibility version to check for.
* @return <code>true</code> if gluster support is enabled, <code>false</code> if it's not.
*/
public static boolean gluster(Version version) {
return SupportedFeatures.GLUSTER.isSupportedOn(version);
}
/**
* @param version
* Compatibility version to check for.
* @return <code>true</code> if gluster heavyweight refresh is enabled, <code>false</code> if it's not.
*/
public static boolean refreshHeavyWeight(Version version) {
return SupportedFeatures.REFRESH_HEAVYWEIGHT.isSupportedOn(version);
}
/* More methods... */
enum SupportedFeatures {
GLUSTER(Version.v3_0),
REFRESH_HEAVYWEIGHT(Version.v3_0, Version.v3_1),
/* More members */;
private Set<Version> unsupportedVersions = new HashSet<Version>();
private SupportedFeatures(Version... versions) {
unsupportedVersions.addAll(Arrays.asList(versions));
}
public boolean isSupportedOn(Version version) {
return !unsupportedVersions.contains(version);
}
}
------------------------------------- END EXAMPLE -------------------------------------
Thoughts?
unless i didn't understand something, this is not good,
this should stay configurable by the users,
for example if some user experience some issues with a feature and want to turn it off/change the values..
(not all version configuration are boolean, some are different values to different versions, like cpu-list)
This is for API level compatibility. If VDSM doesn't support for example hot plug in 3.1 then the user can't just decide that it does and change it. Also, this is not changeable by user since it's not exposed by engine-config (nor should it be).
some are exposed
This is strictly for the engine-VDSM API compatibility, not for other configs which are version specific.
right, but still user should be able to turn features off in case of problems, or change in some cases (for example it is possible to add support for more power management devices, i know it was done by users) no reason to block this
Regards,
Mike
Please share your thoughts on this.
Thanks,
Kanagaraj
_______________________________________________
Engine-devel mailing list
Engine-devel@ovirt.org
------=_Part_21094117_1742490232.1385888972528 Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: quoted-printable <html><body><div style=3D"font-family: times new roman, new york, times, se= rif; font-size: 12pt; color: #000000"><div><br></div><div><br></div><hr id= =3D"zwchr"><blockquote style=3D"border-left:2px solid #1010FF;margin-left:5= px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-de= coration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><b>Fr= om: </b>"Mike Kolesnik" <mkolesni@redhat.com><br><b>To: </b>"Omer Fre= nkel" <ofrenkel@redhat.com><br><b>Cc: </b>"Kanagaraj" <kmayilsa@re= dhat.com>, "engine-devel" <engine-devel@ovirt.org><br><b>Sent: </b=
Sunday, December 1, 2013 11:01:50 AM<br><b>Subject: </b>Re: [Engine-devel]= Using config values<br><div><br></div><div style=3D"font-family: times new= roman, new york, times, serif; font-size: 12pt; color: #000000"><hr id=3D"= zwchr"><blockquote style=3D"border-left:2px solid #1010FF;margin-left:5px;p= adding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decora= tion:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><div styl= e=3D"font-family: times new roman, new york, times, serif; font-size: 12pt;=
color: #000000"><div><br></div><div><br></div><hr id=3D"zwchr"><blockquote= style=3D"border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;co= lor:#000;font-weight:normal;font-style:normal;text-decoration:none;font-fam= ily:Helvetica,Arial,sans-serif;font-size:12pt;"><b>From: </b>"Mike Kolesnik= " <mkolesni@redhat.com><br><b>To: </b>"Kanagaraj" <kmayilsa@redhat= .com><br><b>Cc: </b>"engine-devel" <engine-devel@ovirt.org><br><b>= Sent: </b>Sunday, December 1, 2013 8:08:42 AM<br><b>Subject: </b>Re: [Engin= e-devel] Using config values<br><div><br></div><div style=3D"font-family: t= imes new roman, new york, times, serif; font-size: 12pt; color: #000000"><h= r id=3D"zwchr"><blockquote style=3D"border-left:2px solid #1010FF;margin-le= ft:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;tex= t-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;">H= i All,</blockquote><div><br></div><div>Hi Kanagaraj,<br></div><div><br></di= v><blockquote style=3D"border-left:2px solid #1010FF;margin-left:5px;paddin= g-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:= none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><br> The are s= ome issues arising in configurations whenever we move up on the versions(3.= 3 =3D> 3.4), because of the way we store and interpret them.<br><div><br= ></div>Whenever there is a new cluster level, you will need to add a new en= try 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.<br> Better opt= ion would be to have the defaul config value in ConfigValues.java and the o= verrides will go to config.sql. In this approach you don't need a new entri= es to config.sql when there is a new cluster level.<br><div><br></div>Lets = take an exmaple, "SupportForceCreateVG" - This is supported from 3.1 onward= s,<br><div><br></div>If you look at config.sql, you will see following entr= ies <br> select fn_db_add_config_value('SupportForceCreateVG','false','3.0'= ); <br> select fn_db_add_config_value('SupportForceCreateVG','true','3.1');= <br> select fn_db_add_config_value('SupportForceCreateVG','true','3.2'); <= br> select fn_db_add_config_value('SupportForceCreateVG','true','3.3');<br>= <div><br></div>And in ConfigValues.java<br><div><br></div>  = ; @TypeConverterAttribute(Boolean.class)<br> @DefaultVal= ueAttribute("false")<br> SupportForceCreateVG,<br><div><= br></div>Now if there is 3.4 and 3.5, the user needs to add 2 more entries,= which i feel is redundant.<br><div><br></div>Instead we can make <br><div>= <br></div> @TypeConverterAttribute(Boolean.class)<br> &nb= sp; @DefaultValueAttribute("true")<br> Suppo= rtForceCreateVG,<br><div><br></div>and have only the following in config.sq= l<br> select fn_db_add_config_value('SupportForceCreateVG','false','3.0');<= br><div><br></div>if a particular value(for a specific cluster level) is no= t found in Config.sql, the fallback is to use the value available in Config= Values.java.</blockquote><div><br></div><div>This has already been implemen= ted, there are many "feature supported" configurations working like this (f= or example GlusterSupport).<br></div><div><br></div><div>I think a more int= eresting approach would be to move these out of the DB since these values d= on't really hav e a reson to be there.<br></div><div>Since the entire thing= is abstracted by "Gluster/FeatureSupported" classes then we can easily cha= nge mechanism (of course whatever code is not using it can be easily conver= ted to use the mechanism)<br></div><div><br></div><div>For example a simple= enum could do the trick:<br></div><div>-----------------------------------= -- EXAMPLE -------------------------------------<br></div><div>/**<br> = ;* Convenience class to check if a gluster feature is supported or not in a= ny given version.<br><br> * Methods should be named by feature a= nd accept version to check against.<br> */<br>public class GlusterFeat= ureSupported {</div><div> /**<br> = * @param version<br> * &nbs= p; Compatibility version to check for.<= br> * @return <code>true</code> if glus= ter support is enabled, <code>false</code> if it's not.<br>&nbs= p; */<br> public static boolean gluster= (Version version) {<br> return Su= pportedFeatures.GLUSTER.isSupportedOn(version);<br> }<br>= <div><br></div> /**<br> * @param = version<br> * &n= bsp; Compatibility version to check for.<br> &= nbsp; * @return <code>true</code> if gluster heavyw= eight refresh is enabled, <code>false</code> if it's not.<br>&n= bsp; */<br> public static boolean refre= shHeavyWeight(Version version) {<br> &nb= sp; return SupportedFeatures.REFRESH_HEAVYWEIGHT.isSupportedOn(version);<br= > }<br></div><div><br></div><div> /* Mo= re methods... */</div><div><br> enum SupportedFeatures {<= br> GLUSTER(Version.v3_0),<br>&nb= sp; REFRESH_HEAVYWEIGHT(Version.v3_0, V= ersion.v3_1),</div><div> /* More = members */;<br><div><br></div> pr= ivate Set<Version> unsupportedVersions =3D new HashSet<Version>= ();<br><div><br></div> private Su= pportedFeatures(Version... versions) {<br> &nb= sp; unsupportedVersions.addAll(Arrays.asList(= versions));<br> }<br><div><br></d= iv> public boolean isSupportedOn(= Version version) {<br>  = ; return !unsupportedVersions.contains(version);<br>  = ; }<br> }</div><div>-------= ------------------------------ END EXAMPLE --------------------------------= -----</div><div><br></div><div>Thoughts?</div></div></blockquote><div><br>u= nless i didn't understand something, this is not good,<br></div><div>this s= hould stay configurable by the users,<br></div><div>for example if some use= r experience some issues with a feature and want to turn it off/change the = values..</div><div>(not all version configuration are boolean, some are dif= ferent values to different versions, like cpu-list)</div></div></blockquote= ><div><br></div><div>This is for API level compatibility.<br></div><div>If = VDSM doesn't support for example hot plug in 3.1 then the user can't just d= ecide that it does and change it.<br></div><div>Also, this is not changeabl= e by user since it's not exposed by engine-config (nor should it be).<br></= div></div></blockquote><div>some are exposed<br></div><blockquote style=3D"= border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;f= ont-weight:normal;font-style:normal;text-decoration:none;font-family:Helvet= ica,Arial,sans-serif;font-size:12pt;"><div style=3D"font-family: times new = roman, new york, times, serif; font-size: 12pt; color: #000000"><div>This i= s strictly for the engine-VDSM API compatibility, not for other configs whi= ch are version specific.<br></div></div></blockquote><div>right, but still = user should be able to turn features off in case of problems,<br></div><div= >or change in some cases (for example it is possible to add support for mor= e power management devices, i know it was done by users)<br></div><div>no r= eason to block this<br></div><blockquote style=3D"border-left:2px solid #10= 10FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-st= yle:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font= -size:12pt;"><div style=3D"font-family: times new roman, new york, times, s= erif; font-size: 12pt; color: #000000"><blockquote style=3D"border-left:2px= solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:norm= al;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-= serif;font-size:12pt;"><div style=3D"font-family: times new roman, new york= , times, serif; font-size: 12pt; color: #000000"><div><br></div><blockquote= style=3D"border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;co= lor:#000;font-weight:normal;font-style:normal;text-decoration:none;font-fam= ily:Helvetica,Arial,sans-serif;font-size:12pt;"><div style=3D"font-family: = times new roman, new york, times, serif; font-size: 12pt; color: #000000"><= div><br></div><div><br></div><div>Regards,<br></div><div>Mike<br></div><blo= ckquote style=3D"border-left:2px solid #1010FF;margin-left:5px;padding-left= :5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;f= ont-family:Helvetica,Arial,sans-serif;font-size:12pt;"><br> Please share yo= ur thoughts on this.<br><div><br></div>Thanks,<br> Kanagaraj<br><div><br></= div></blockquote></div><br>_______________________________________________<= br>Engine-devel mailing list<br>Engine-devel@ovirt.org<br>http://lists.ovir= t.org/mailman/listinfo/engine-devel<br></blockquote><div><br></div></div></= blockquote><div><br></div></div></blockquote><div><br></div></div></body></= html> ------=_Part_21094117_1742490232.1385888972528--
----- Original Message -----
From: "Omer Frenkel" <ofrenkel@redhat.com> To: "Mike Kolesnik" <mkolesni@redhat.com> Cc: "engine-devel" <engine-devel@ovirt.org> Sent: Sunday, December 1, 2013 11:09:32 AM Subject: Re: [Engine-devel] Using config values
From: "Mike Kolesnik" <mkolesni@redhat.com> To: "Omer Frenkel" <ofrenkel@redhat.com> Cc: "Kanagaraj" <kmayilsa@redhat.com>, "engine-devel" <engine-devel@ovirt.org> Sent: Sunday, December 1, 2013 11:01:50 AM Subject: Re: [Engine-devel] Using config values
From: "Mike Kolesnik" <mkolesni@redhat.com> To: "Kanagaraj" <kmayilsa@redhat.com> Cc: "engine-devel" <engine-devel@ovirt.org> Sent: Sunday, December 1, 2013 8:08:42 AM Subject: Re: [Engine-devel] Using config values
Hi All,
Hi Kanagaraj,
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.
This has already been implemented, there are many "feature supported" configurations working like this (for example GlusterSupport).
I think a more interesting approach would be to move these out of the DB since these values don't really hav e a reson to be there. Since the entire thing is abstracted by "Gluster/FeatureSupported" classes then we can easily change mechanism (of course whatever code is not using it can be easily converted to use the mechanism)
For example a simple enum could do the trick: ------------------------------------- EXAMPLE ------------------------------------- /** * Convenience class to check if a gluster feature is supported or not in any given version.<br> * Methods should be named by feature and accept version to check against. */ public class GlusterFeatureSupported { /** * @param version * Compatibility version to check for. * @return <code>true</code> if gluster support is enabled, <code>false</code> if it's not. */ public static boolean gluster(Version version) { return SupportedFeatures.GLUSTER.isSupportedOn(version); }
/** * @param version * Compatibility version to check for. * @return <code>true</code> if gluster heavyweight refresh is enabled, <code>false</code> if it's not. */ public static boolean refreshHeavyWeight(Version version) { return SupportedFeatures.REFRESH_HEAVYWEIGHT.isSupportedOn(version); }
/* More methods... */
enum SupportedFeatures { GLUSTER(Version.v3_0), REFRESH_HEAVYWEIGHT(Version.v3_0, Version.v3_1), /* More members */;
private Set<Version> unsupportedVersions = new HashSet<Version>();
private SupportedFeatures(Version... versions) { unsupportedVersions.addAll(Arrays.asList(versions)); }
public boolean isSupportedOn(Version version) { return !unsupportedVersions.contains(version); } } ------------------------------------- END EXAMPLE -------------------------------------
Thoughts?
unless i didn't understand something, this is not good, this should stay configurable by the users, for example if some user experience some issues with a feature and want to turn it off/change the values.. (not all version configuration are boolean, some are different values to different versions, like cpu-list)
This is for API level compatibility. If VDSM doesn't support for example hot plug in 3.1 then the user can't just decide that it does and change it. Also, this is not changeable by user since it's not exposed by engine-config (nor should it be). some are exposed
This is strictly for the engine-VDSM API compatibility, not for other configs which are version specific. right, but still user should be able to turn features off in case of problems, or change in some cases (for example it is possible to add support for more power management devices, i know it was done by users) no reason to block this
In this case you add additional protection level to block user from enabling features for unsupported cluster levels. However it sounds more reasonable than requiring the user to use a lower cluster level which enforces him to give up on other features.
Regards, Mike
Please share your thoughts on this.
Thanks, Kanagaraj
_______________________________________________ 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
On 12/01/2013 11:23 AM, Moti Asayag wrote:
----- Original Message -----
From: "Omer Frenkel" <ofrenkel@redhat.com> To: "Mike Kolesnik" <mkolesni@redhat.com> Cc: "engine-devel" <engine-devel@ovirt.org> Sent: Sunday, December 1, 2013 11:09:32 AM Subject: Re: [Engine-devel] Using config values
From: "Mike Kolesnik" <mkolesni@redhat.com> To: "Omer Frenkel" <ofrenkel@redhat.com> Cc: "Kanagaraj" <kmayilsa@redhat.com>, "engine-devel" <engine-devel@ovirt.org> Sent: Sunday, December 1, 2013 11:01:50 AM Subject: Re: [Engine-devel] Using config values
From: "Mike Kolesnik" <mkolesni@redhat.com> To: "Kanagaraj" <kmayilsa@redhat.com> Cc: "engine-devel" <engine-devel@ovirt.org> Sent: Sunday, December 1, 2013 8:08:42 AM Subject: Re: [Engine-devel] Using config values
Hi All,
Hi Kanagaraj,
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.
This has already been implemented, there are many "feature supported" configurations working like this (for example GlusterSupport).
I think a more interesting approach would be to move these out of the DB since these values don't really hav e a reson to be there. Since the entire thing is abstracted by "Gluster/FeatureSupported" classes then we can easily change mechanism (of course whatever code is not using it can be easily converted to use the mechanism)
For example a simple enum could do the trick: ------------------------------------- EXAMPLE ------------------------------------- /** * Convenience class to check if a gluster feature is supported or not in any given version.<br> * Methods should be named by feature and accept version to check against. */ public class GlusterFeatureSupported { /** * @param version * Compatibility version to check for. * @return <code>true</code> if gluster support is enabled, <code>false</code> if it's not. */ public static boolean gluster(Version version) { return SupportedFeatures.GLUSTER.isSupportedOn(version); }
/** * @param version * Compatibility version to check for. * @return <code>true</code> if gluster heavyweight refresh is enabled, <code>false</code> if it's not. */ public static boolean refreshHeavyWeight(Version version) { return SupportedFeatures.REFRESH_HEAVYWEIGHT.isSupportedOn(version); }
/* More methods... */
enum SupportedFeatures { GLUSTER(Version.v3_0), REFRESH_HEAVYWEIGHT(Version.v3_0, Version.v3_1), /* More members */;
private Set<Version> unsupportedVersions = new HashSet<Version>();
private SupportedFeatures(Version... versions) { unsupportedVersions.addAll(Arrays.asList(versions)); }
public boolean isSupportedOn(Version version) { return !unsupportedVersions.contains(version); } } ------------------------------------- END EXAMPLE -------------------------------------
Thoughts?
unless i didn't understand something, this is not good, this should stay configurable by the users, for example if some user experience some issues with a feature and want to turn it off/change the values.. (not all version configuration are boolean, some are different values to different versions, like cpu-list)
This is for API level compatibility. If VDSM doesn't support for example hot plug in 3.1 then the user can't just decide that it does and change it. Also, this is not changeable by user since it's not exposed by engine-config (nor should it be). some are exposed
This is strictly for the engine-VDSM API compatibility, not for other configs which are version specific. right, but still user should be able to turn features off in case of problems, or change in some cases (for example it is possible to add support for more power management devices, i know it was done by users) no reason to block this
In this case you add additional protection level to block user from enabling features for unsupported cluster levels.
However it sounds more reasonable than requiring the user to use a lower cluster level which enforces him to give up on other features.
lowering the cluster level is not supported. there is a difference between the configs we expose to users, and the configs we prefer to not be changed, but are used to workaround issues in some cases.
------=_Part_43208504_1304306350.1385889991982 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Regards, Mike ----- Original Message -----
----- Original Message -----
From: "Mike Kolesnik" <mkolesni@redhat.com>
To: "Omer Frenkel" <ofrenkel@redhat.com>
Cc: "Kanagaraj" <kmayilsa@redhat.com>, "engine-devel" <engine-devel@ovirt.org>
Sent: Sunday, December 1, 2013 11:01:50 AM
Subject: Re: [Engine-devel] Using config values
----- Original Message -----
----- Original Message -----
From: "Mike Kolesnik" <mkolesni@redhat.com>
To: "Kanagaraj" <kmayilsa@redhat.com>
Cc: "engine-devel" <engine-devel@ovirt.org>
Sent: Sunday, December 1, 2013 8:08:42 AM
Subject: Re: [Engine-devel] Using config values
----- Original Message -----
Hi All,
Hi Kanagaraj,
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.
This has already been implemented, there are many "feature supported" configurations working like this (for example GlusterSupport).
I think a more interesting approach would be to move these out of the DB since these values don't really hav e a reson to be there.
Since the entire thing is abstracted by "Gluster/FeatureSupported" classes then we can easily change mechanism (of course whatever code is not using it can be easily converted to use the mechanism)
For example a simple enum could do the trick:
------------------------------------- EXAMPLE -------------------------------------
/**
* Convenience class to check if a gluster feature is supported or not in any given version.<br>
* Methods should be named by feature and accept version to check against.
*/
public class GlusterFeatureSupported {
/**
* @param version
* Compatibility version to check for.
* @return <code>true</code> if gluster support is enabled, <code>false</code> if it's not.
*/
public static boolean gluster(Version version) {
return SupportedFeatures.GLUSTER.isSupportedOn(version);
}
/**
* @param version
* Compatibility version to check for.
* @return <code>true</code> if gluster heavyweight refresh is enabled, <code>false</code> if it's not.
*/
public static boolean refreshHeavyWeight(Version version) {
return SupportedFeatures.REFRESH_HEAVYWEIGHT.isSupportedOn(version);
}
/* More methods... */
enum SupportedFeatures {
GLUSTER(Version.v3_0),
REFRESH_HEAVYWEIGHT(Version.v3_0, Version.v3_1),
/* More members */;
private Set<Version> unsupportedVersions = new HashSet<Version>();
private SupportedFeatures(Version... versions) {
unsupportedVersions.addAll(Arrays.asList(versions));
}
public boolean isSupportedOn(Version version) {
return !unsupportedVersions.contains(version);
}
}
------------------------------------- END EXAMPLE -------------------------------------
Thoughts?
unless i didn't understand something, this is not good,
this should stay configurable by the users,
for example if some user experience some issues with a feature and want to turn it off/change the values..
(not all version configuration are boolean, some are different values to different versions, like cpu-list)
This is for API level compatibility.
If VDSM doesn't support for example hot plug in 3.1 then the user can't just decide that it does and change it.
Also, this is not changeable by user since it's not exposed by engine-config (nor should it be).
some are exposed
If there is such a thing then its a design flaw. The only one I see in FeatureSupported class is EnableMACAntiSpoofingFilterRules This should be split into 2: MACAntiSpoofingFilterRulesSupported - which is used to determine API compatibility EnableMACAntiSpoofingFilterRules - which the user can set to determine system behaviour I see no reason why to allow user to set EnableMACAntiSpoofingFilterRules=true for 3.0 since it will not work so it's just confusing to him
This is strictly for the engine-VDSM API compatibility, not for other configs which are version specific.
right, but still user should be able to turn features off in case of problems, or change in some cases (for example it is possible to add support for more power management devices, i know it was done by users) no reason to block this
Again not talking about all the config values, strictly the ones that are (or should be) in FeatureSupported class. In case we want to allow user to tweak behavior there, we can always do as I suggested for EnableMACAntiSpoofingFilterRules
Regards,
Mike
Please share your thoughts on this.
Thanks,
Kanagaraj
_______________________________________________
Engine-devel mailing list
Engine-devel@ovirt.org
------=_Part_43208504_1304306350.1385889991982 Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: quoted-printable <html><body><div style=3D"font-family: times new roman, new york, times, se= rif; font-size: 12pt; color: #000000"><div><br></div><div><br></div><div><s= pan name=3D"x"></span>Regards,<br>Mike<span name=3D"x"></span><br></div><di= v><br></div><hr id=3D"zwchr"><blockquote style=3D"border-left:2px solid #10= 10FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-st= yle:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font= -size:12pt;"><div style=3D"font-family: times new roman, new york, times, s= erif; font-size: 12pt; color: #000000"><div><br></div><div><br></div><hr id= =3D"zwchr"><blockquote style=3D"border-left:2px solid #1010FF;margin-left:5= px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-de= coration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><b>Fr= om: </b>"Mike Kolesnik" <mkolesni@redhat.com><br><b>To: </b>"Omer Fre= nkel" <ofrenkel@redhat.com><br><b>Cc: </b>"Kanagaraj" <kmayilsa@re= dhat.com>, "engine-devel" <engine-devel@ovirt.org><br><b>Sent: </b=
Sunday, December 1, 2013 11:01:50 AM<br><b>Subject: </b>Re: [Engine-devel]= Using config values<br><div><br></div><div style=3D"font-family: times new= roman, new york, times, serif; font-size: 12pt; color: #000000"><hr id=3D"= zwchr"><blockquote style=3D"border-left:2px solid #1010FF;margin-left:5px;p= adding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decora= tion:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><div styl= e=3D"font-family: times new roman, new york, times, serif; font-size: 12pt;=
color: #000000"><div><br></div><div><br></div><hr id=3D"zwchr"><blockquote= style=3D"border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;co= lor:#000;font-weight:normal;font-style:normal;text-decoration:none;font-fam= ily:Helvetica,Arial,sans-serif;font-size:12pt;"><b>From: </b>"Mike Kolesnik= " <mkolesni@redhat.com><br><b>To: </b>"Kanagaraj" <kmayilsa@redhat= .com><br><b>Cc: </b>"engine-devel" <engine-devel@ovirt.org><br><b>= Sent: </b>Sunday, December 1, 2013 8:08:42 AM<br><b>Subject: </b>Re: [Engin= e-devel] Using config values<br><div><br></div><div style=3D"font-family: t= imes new roman, new york, times, serif; font-size: 12pt; color: #000000"><h= r id=3D"zwchr"><blockquote style=3D"border-left:2px solid #1010FF;margin-le= ft:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;tex= t-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;">H= i All,</blockquote><div><br></div><div>Hi Kanagaraj,<br></div><div><br></di= v><blockquote style=3D"border-left:2px solid #1010FF;margin-left:5px;paddin= g-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:= none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><br> The are s= ome issues arising in configurations whenever we move up on the versions(3.= 3 =3D> 3.4), because of the way we store and interpret them.<br><div><br= ></div>Whenever there is a new cluster level, you will need to add a new en= try 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.<br> Better opt= ion would be to have the defaul config value in ConfigValues.java and the o= verrides will go to config.sql. In this approach you don't need a new entri= es to config.sql when there is a new cluster level.<br><div><br></div>Lets = take an exmaple, "SupportForceCreateVG" - This is supported from 3.1 onward= s,<br><div><br></div>If you look at config.sql, you will see following entr= ies <br> select fn_db_add_config_value('SupportForceCreateVG','false','3.0'= ); <br> select fn_db_add_config_value('SupportForceCreateVG','true','3.1');= <br> select fn_db_add_config_value('SupportForceCreateVG','true','3.2'); <= br> select fn_db_add_config_value('SupportForceCreateVG','true','3.3');<br>= <div><br></div>And in ConfigValues.java<br><div><br></div>  = ; @TypeConverterAttribute(Boolean.class)<br> @DefaultVal= ueAttribute("false")<br> SupportForceCreateVG,<br><div><= br></div>Now if there is 3.4 and 3.5, the user needs to add 2 more entries,= which i feel is redundant.<br><div><br></div>Instead we can make <br><div>= <br></div> @TypeConverterAttribute(Boolean.class)<br> &nb= sp; @DefaultValueAttribute("true")<br> Suppo= rtForceCreateVG,<br><div><br></div>and have only the following in config.sq= l<br> select fn_db_add_config_value('SupportForceCreateVG','false','3.0');<= br><div><br></div>if a particular value(for a specific cluster level) is no= t found in Config.sql, the fallback is to use the value available in Config= Values.java.</blockquote><div><br></div><div>This has already been implemen= ted, there are many "feature supported" configurations working like this (f= or example GlusterSupport).<br></div><div><br></div><div>I think a more int= eresting approach would be to move these out of the DB since these values d= on't really hav e a reson to be there.<br></div><div>Since the entire thing= is abstracted by "Gluster/FeatureSupported" classes then we can easily cha= nge mechanism (of course whatever code is not using it can be easily conver= ted to use the mechanism)<br></div><div><br></div><div>For example a simple= enum could do the trick:<br></div><div>-----------------------------------= -- EXAMPLE -------------------------------------<br></div><div>/**<br> = ;* Convenience class to check if a gluster feature is supported or not in a= ny given version.<br><br> * Methods should be named by feature a= nd accept version to check against.<br> */<br>public class GlusterFeat= ureSupported {</div><div> /**<br> = * @param version<br> * &nbs= p; Compatibility version to check for.<= br> * @return <code>true</code> if glus= ter support is enabled, <code>false</code> if it's not.<br>&nbs= p; */<br> public static boolean gluster= (Version version) {<br> return Su= pportedFeatures.GLUSTER.isSupportedOn(version);<br> }<br>= <div><br></div> /**<br> * @param = version<br> * &n= bsp; Compatibility version to check for.<br> &= nbsp; * @return <code>true</code> if gluster heavyw= eight refresh is enabled, <code>false</code> if it's not.<br>&n= bsp; */<br> public static boolean refre= shHeavyWeight(Version version) {<br> &nb= sp; return SupportedFeatures.REFRESH_HEAVYWEIGHT.isSupportedOn(version);<br= > }<br></div><div><br></div><div> /* Mo= re methods... */</div><div><br> enum SupportedFeatures {<= br> GLUSTER(Version.v3_0),<br>&nb= sp; REFRESH_HEAVYWEIGHT(Version.v3_0, V= ersion.v3_1),</div><div> /* More = members */;<br><div><br></div> pr= ivate Set<Version> unsupportedVersions =3D new HashSet<Version>= ();<br><div><br></div> private Su= pportedFeatures(Version... versions) {<br> &nb= sp; unsupportedVersions.addAll(Arrays.asList(= versions));<br> }<br><div><br></d= iv> public boolean isSupportedOn(= Version version) {<br>  = ; return !unsupportedVersions.contains(version);<br>  = ; }<br> }</div><div>-------= ------------------------------ END EXAMPLE --------------------------------= -----</div><div><br></div><div>Thoughts?</div></div></blockquote><div><br>u= nless i didn't understand something, this is not good,<br></div><div>this s= hould stay configurable by the users,<br></div><div>for example if some use= r experience some issues with a feature and want to turn it off/change the = values..</div><div>(not all version configuration are boolean, some are dif= ferent values to different versions, like cpu-list)</div></div></blockquote= ><div><br></div><div>This is for API level compatibility.<br></div><div>If = VDSM doesn't support for example hot plug in 3.1 then the user can't just d= ecide that it does and change it.<br></div><div>Also, this is not changeabl= e by user since it's not exposed by engine-config (nor should it be).<br></= div></div></blockquote><div>some are exposed</div></div></blockquote><div>I= f there is such a thing then its a design flaw.<br></div><div><br></div><di= v>The only one I see in FeatureSupported class is EnableMACAntiSpoofingFilt= erRules<br></div><div>This should be split into 2:<br></div><div>MACAntiSpo= ofingFilterRulesSupported - which is used to determine API compatibility<br= ></div><div>EnableMACAntiSpoofingFilterRules - which the user can set to de= termine system behaviour<br></div><div><br></div><div>I see no reason why t= o allow user to set EnableMACAntiSpoofingFilterRules=3Dtrue for 3.0 since i= t will not work so it's just confusing to him<br></div><blockquote style=3D= "border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;= font-weight:normal;font-style:normal;text-decoration:none;font-family:Helve= tica,Arial,sans-serif;font-size:12pt;"><div style=3D"font-family: times new= roman, new york, times, serif; font-size: 12pt; color: #000000"><div><br><= /div><blockquote style=3D"border-left:2px solid #1010FF;margin-left:5px;pad= ding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decorati= on:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><div style= =3D"font-family: times new roman, new york, times, serif; font-size: 12pt; = color: #000000"><div>This is strictly for the engine-VDSM API compatibility= , not for other configs which are version specific.<br></div></div></blockq= uote><div>right, but still user should be able to turn features off in case= of problems,<br></div><div>or change in some cases (for example it is poss= ible to add support for more power management devices, i know it was done b= y users)<br></div><div>no reason to block this</div></div></blockquote><div= >Again not talking about all the config values, strictly the ones that are = (or should be) in FeatureSupported class.<br></div><div><br></div><div>In c= ase we want to allow user to tweak behavior there, we can always do as I su= ggested for EnableMACAntiSpoofingFilterRules<br></div><blockquote style=3D"= border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;f= ont-weight:normal;font-style:normal;text-decoration:none;font-family:Helvet= ica,Arial,sans-serif;font-size:12pt;"><div style=3D"font-family: times new = roman, new york, times, serif; font-size: 12pt; color: #000000"><div><br></= div><blockquote style=3D"border-left:2px solid #1010FF;margin-left:5px;padd= ing-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoratio= n:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><div style= =3D"font-family: times new roman, new york, times, serif; font-size: 12pt; = color: #000000"><blockquote style=3D"border-left:2px solid #1010FF;margin-l= eft:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;te= xt-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;">= <div style=3D"font-family: times new roman, new york, times, serif; font-si= ze: 12pt; color: #000000"><div><br></div><blockquote style=3D"border-left:2= px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:no= rmal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,san= s-serif;font-size:12pt;"><div style=3D"font-family: times new roman, new yo= rk, times, serif; font-size: 12pt; color: #000000"><div><br></div><div><br>= </div><div>Regards,<br></div><div>Mike<br></div><blockquote style=3D"border= -left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-we= ight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Ar= ial,sans-serif;font-size:12pt;"><br> Please share your thoughts on this.<br= ><div><br></div>Thanks,<br> Kanagaraj<br><div><br></div></blockquote></div>= <br>_______________________________________________<br>Engine-devel mailing= list<br>Engine-devel@ovirt.org<br>http://lists.ovirt.org/mailman/listinfo/= engine-devel<br></blockquote><div><br></div></div></blockquote><div><br></d= iv></div></blockquote><div><br></div></div></blockquote><div><br></div></di= v></body></html> ------=_Part_43208504_1304306350.1385889991982--
participants (8)
-
Dusmant Kumar Pati -
Eli Mesika -
Itamar Heim -
Kanagaraj -
Mike Kolesnik -
Moti Asayag -
Omer Frenkel -
Sahina Bose