2:16 a.m.
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--
3:06 a.m.
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(a)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">...
</pre>
</blockquote>
<br>
</body>
</html>
--------------060202090604020609010600--
5:40 a.m.
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(a)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">...
</pre>
</blockquote>
I think, this is a good suggestion...<br>
<br>
</body>
</html>
--------------010309070708090904000001--
2:58 p.m.
----- Original Message -----
From: "Dusmant Kumar Pati" <dpati(a)redhat.com>
To: "Kanagaraj" <kmayilsa(a)redhat.com>, "engine-devel"
<engine-devel(a)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(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
I think, this is a good suggestion...
_______________________________________________
Engine-devel mailing list
Engine-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
3:02 a.m.
----- Original Message -----
From: "Eli Mesika" <emesika(a)redhat.com>
To: "Dusmant Kumar Pati" <dpati(a)redhat.com>
Cc: "engine-devel" <engine-devel(a)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(a)redhat.com>
> To: "Kanagaraj" <kmayilsa(a)redhat.com>, "engine-devel"
> <engine-devel(a)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(a)ovirt.org
> http://lists.ovirt.org/mailman/listinfo/engine-devel
> I think, this is a good suggestion...
>
>
> _______________________________________________
> Engine-devel mailing list
> Engine-devel(a)ovirt.org
> http://lists.ovirt.org/mailman/listinfo/engine-devel
>
_______________________________________________
Engine-devel mailing list
Engine-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
3:58 a.m.
----- Original Message -----
From: "Omer Frenkel" <ofrenkel(a)redhat.com>
To: "Eli Mesika" <emesika(a)redhat.com>
Cc: "Dusmant Kumar Pati" <dpati(a)redhat.com>, "engine-devel"
<engine-devel(a)ovirt.org>
Sent: Sunday, December 1, 2013 11:02:46 AM
Subject: Re: [Engine-devel] Using config values
----- Original Message -----
> From: "Eli Mesika" <emesika(a)redhat.com>
> To: "Dusmant Kumar Pati" <dpati(a)redhat.com>
> Cc: "engine-devel" <engine-devel(a)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(a)redhat.com>
> > To: "Kanagaraj" <kmayilsa(a)redhat.com>,
"engine-devel"
> > <engine-devel(a)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(a)ovirt.org
> > http://lists.ovirt.org/mailman/listinfo/engine-devel
> > I think, this is a good suggestion...
> >
> >
> > _______________________________________________
> > Engine-devel mailing list
> > Engine-devel(a)ovirt.org
> > http://lists.ovirt.org/mailman/listinfo/engine-devel
> >
> _______________________________________________
> Engine-devel mailing list
> Engine-devel(a)ovirt.org
> http://lists.ovirt.org/mailman/listinfo/engine-devel
>
10:35 a.m.
On 12/01/2013 11:58 AM, Eli Mesika wrote:
----- Original Message -----
> From: "Omer Frenkel" <ofrenkel(a)redhat.com>
> To: "Eli Mesika" <emesika(a)redhat.com>
> Cc: "Dusmant Kumar Pati" <dpati(a)redhat.com>, "engine-devel"
<engine-devel(a)ovirt.org>
> Sent: Sunday, December 1, 2013 11:02:46 AM
> Subject: Re: [Engine-devel] Using config values
>
>
>
> ----- Original Message -----
>> From: "Eli Mesika" <emesika(a)redhat.com>
>> To: "Dusmant Kumar Pati" <dpati(a)redhat.com>
>> Cc: "engine-devel" <engine-devel(a)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(a)redhat.com>
>>> To: "Kanagaraj" <kmayilsa(a)redhat.com>,
"engine-devel"
>>> <engine-devel(a)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.
12:08 a.m.
------=_Part_43195740_1618344988.1385878122945
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
----- Original Message -----
=20
=20
=20
=20
Hi All,</blockquote><div><br></div><div>Hi
Kanagaraj,<br></div><div><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=
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.</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=
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--
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;"</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
2:55 a.m.
------=_Part_21090047_1204116571.1385888119663
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
----- Original Message -----
=20
=20
=20
=20
Hi All,</blockquote><div><br></div><div>Hi
Kanagaraj,<br></div><div><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<...
uote><div><br></div></div></body></html ------=_Part_21090047_1204116571.1385888119663--
From: "Mike Kolesnik" <mkolesni(a)redhat.com To: "Kanagaraj" <kmayilsa(a)redhat.com Cc: "engine-devel" <engine-devel(a)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(a)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"
&lt;mkolesni(a)redhat.com&gt;<br><b>To: </b>"Kanagara=
j" &lt;kmayilsa(a)redhat.com&gt;<br><b>Cc:
</b>"engine-devel" <engine-deve=
l(a)ovirt.org&gt;<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;"</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
3:01 a.m.
------=_Part_43206801_1748816137.1385888510809
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
----- Original Message -----
----- Original Message -----
> From: "Mike Kolesnik" <mkolesni(a)redhat.com>
> To: "Kanagaraj" <kmayilsa(a)redhat.com>
> Cc: "engine-devel" <engine-devel(a)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(a)ovirt.org
> http://lists.ovirt.org/mailman/listinfo/engine-devel
------=_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=
" &lt;mkolesni(a)redhat.com&gt;<br><b>To:
</b>"Kanagaraj" <kmayilsa@redhat=
.com><br><b>Cc: </b>"engine-devel"
&lt;engine-devel(a)ovirt.org&gt;<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--
3:09 a.m.
------=_Part_21094117_1742490232.1385888972528
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
----- Original Message -----
From: "Mike Kolesnik" <mkolesni(a)redhat.com>
To: "Omer Frenkel" <ofrenkel(a)redhat.com>
Cc: "Kanagaraj" <kmayilsa(a)redhat.com>, "engine-devel"
<engine-devel(a)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(a)redhat.com>
>
> > To: "Kanagaraj" <kmayilsa(a)redhat.com>
>
> > Cc: "engine-devel" <engine-devel(a)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(a)ovirt.org
>
> > http://lists.ovirt.org/mailman/listinfo/engine-devel
>
------=_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"
&lt;mkolesni(a)redhat.com&gt;<br><b>To: </b>"Omer Fre=
nkel" &lt;ofrenkel(a)redhat.com&gt;<br><b>Cc:
</b>"Kanagaraj" <kmayilsa@re=
dhat.com>, "engine-devel"
&lt;engine-devel(a)ovirt.org&gt;<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=
" &lt;mkolesni(a)redhat.com&gt;<br><b>To:
</b>"Kanagaraj" <kmayilsa@redhat=
.com><br><b>Cc: </b>"engine-devel"
&lt;engine-devel(a)ovirt.org&gt;<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--
3:23 a.m.
----- Original Message -----
From: "Omer Frenkel" <ofrenkel(a)redhat.com>
To: "Mike Kolesnik" <mkolesni(a)redhat.com>
Cc: "engine-devel" <engine-devel(a)ovirt.org>
Sent: Sunday, December 1, 2013 11:09:32 AM
Subject: Re: [Engine-devel] Using config values
From: "Mike Kolesnik" <mkolesni(a)redhat.com>
To: "Omer Frenkel" <ofrenkel(a)redhat.com>
Cc: "Kanagaraj" <kmayilsa(a)redhat.com>, "engine-devel"
<engine-devel(a)ovirt.org>
Sent: Sunday, December 1, 2013 11:01:50 AM
Subject: Re: [Engine-devel] Using config values
From: "Mike Kolesnik" <mkolesni(a)redhat.com>
To: "Kanagaraj" <kmayilsa(a)redhat.com>
Cc: "engine-devel" <engine-devel(a)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(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
_______________________________________________
Engine-devel mailing list
Engine-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
10:33 a.m.
On 12/01/2013 11:23 AM, Moti Asayag wrote:
----- Original Message -----
> From: "Omer Frenkel" <ofrenkel(a)redhat.com>
> To: "Mike Kolesnik" <mkolesni(a)redhat.com>
> Cc: "engine-devel" <engine-devel(a)ovirt.org>
> Sent: Sunday, December 1, 2013 11:09:32 AM
> Subject: Re: [Engine-devel] Using config values
>
>
>
>
>
>
> From: "Mike Kolesnik" <mkolesni(a)redhat.com>
> To: "Omer Frenkel" <ofrenkel(a)redhat.com>
> Cc: "Kanagaraj" <kmayilsa(a)redhat.com>, "engine-devel"
> <engine-devel(a)ovirt.org>
> Sent: Sunday, December 1, 2013 11:01:50 AM
> Subject: Re: [Engine-devel] Using config values
>
>
>
>
>
>
>
>
>
>
> From: "Mike Kolesnik" <mkolesni(a)redhat.com>
> To: "Kanagaraj" <kmayilsa(a)redhat.com>
> Cc: "engine-devel" <engine-devel(a)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.
3:26 a.m.
------=_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(a)redhat.com>
> To: "Omer Frenkel" <ofrenkel(a)redhat.com>
> Cc: "Kanagaraj" <kmayilsa(a)redhat.com>, "engine-devel"
> <engine-devel(a)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(a)redhat.com>
> >
>
> > > To: "Kanagaraj" <kmayilsa(a)redhat.com>
> >
>
> > > Cc: "engine-devel" <engine-devel(a)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(a)ovirt.org
> >
>
> > > http://lists.ovirt.org/mailman/listinfo/engine-devel
> >
>
------=_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"
&lt;mkolesni(a)redhat.com&gt;<br><b>To: </b>"Omer Fre=
nkel" &lt;ofrenkel(a)redhat.com&gt;<br><b>Cc:
</b>"Kanagaraj" <kmayilsa@re=
dhat.com>, "engine-devel"
&lt;engine-devel(a)ovirt.org&gt;<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=
" &lt;mkolesni(a)redhat.com&gt;<br><b>To:
</b>"Kanagaraj" <kmayilsa@redhat=
.com><br><b>Cc: </b>"engine-devel"
&lt;engine-devel(a)ovirt.org&gt;<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--
3795
days inactive
3800
days old
13 comments
8 participants
participants (8)
-
Dusmant Kumar Pati -
Eli Mesika -
Itamar Heim -
Kanagaraj -
Mike Kolesnik -
Moti Asayag -
Omer Frenkel -
Sahina Bose