<html><body><div style="font-family: times new roman, new york, times, serif; font-size: 12pt; color: #000000"><div><br></div><div><br></div><hr id="zwchr"><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><b>From: </b>"Mike Kolesnik" <mkolesni@redhat.com><br><b>To: </b>"Omer Frenkel" <ofrenkel@redhat.com><br><b>Cc: </b>"Kanagaraj" <kmayilsa@redhat.com>, "engine-devel" <engine-devel@ovirt.org><br><b>Sent: </b>Sunday, December 1, 2013 11:01:50 AM<br><b>Subject: </b>Re: [Engine-devel] Using config values<br><div><br></div><div style="font-family: times new roman, new york, times, serif; font-size: 12pt; color: #000000"><hr id="zwchr"><blockquote style="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 style="font-family: times new roman, new york, times, serif; font-size: 12pt; color: #000000"><div><br></div><div><br></div><hr id="zwchr"><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><b>From: </b>"Mike Kolesnik" <mkolesni@redhat.com><br><b>To: </b>"Kanagaraj" <kmayilsa@redhat.com><br><b>Cc: </b>"engine-devel" <engine-devel@ovirt.org><br><b>Sent: </b>Sunday, December 1, 2013 8:08:42 AM<br><b>Subject: </b>Re: [Engine-devel] Using config values<br><div><br></div><div style="font-family: times new roman, new york, times, serif; font-size: 12pt; color: #000000"><hr id="zwchr"><blockquote style="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;">Hi All,</blockquote><div><br></div><div>Hi Kanagaraj,<br></div><div><br></div><blockquote style="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;"><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><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 been implemented, there are many "feature supported" configurations 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" classes then we can easily change mechanism (of course whatever code is not using it can be easily converted 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 any given version.<br><br> * Methods should be named by feature and accept version to check against.<br> */<br>public class GlusterFeatureSupported {</div><div> /**<br> * @param version<br> * Compatibility version to check for.<br> * @return <code>true</code> if gluster support is enabled, <code>false</code> if it's not.<br> */<br> public static boolean gluster(Version version) {<br> return SupportedFeatures.GLUSTER.isSupportedOn(version);<br> }<br><div><br></div> /**<br> * @param version<br> * Compatibility version 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><div> /* More methods... */</div><div><br> enum SupportedFeatures {<br> GLUSTER(Version.v3_0),<br> REFRESH_HEAVYWEIGHT(Version.v3_0, Version.v3_1),</div><div> /* More members */;<br><div><br></div> private Set<Version> unsupportedVersions = new HashSet<Version>();<br><div><br></div> private SupportedFeatures(Version... versions) {<br> unsupportedVersions.addAll(Arrays.asList(versions));<br> }<br><div><br></div> 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>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 configuration are boolean, some are different 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 decide that it does and change it.<br></div><div>Also, this is not changeable 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="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 style="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></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 more power management devices, i know it was done by users)<br></div><div>no reason to block this<br></div><blockquote style="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 style="font-family: times new roman, new york, times, serif; font-size: 12pt; color: #000000"><blockquote style="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 style="font-family: times new roman, new york, times, serif; font-size: 12pt; color: #000000"><div><br></div><blockquote style="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 style="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="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;"><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></div></div></blockquote><div><br></div></div></body></html>