10: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