[Engine-devel] FeatureSupported and compatibility versions

[Shireesh Anjal]

On 03/18/2013 01:30 PM, Omer Frenkel wrote:
>
> ----- Original Message -----
>> From: "Shireesh Anjal" <sanjal at redhat.com>
>> To: engine-devel at ovirt.org
>> Sent: Monday, March 18, 2013 8:28:14 AM
>> Subject: [Engine-devel] FeatureSupported and compatibility versions
>>
>> Hi all,
>>
>> The current mechanism in oVirt to check whether a feature is
>> supported
>> in a particular compatibility version is to use the FeatureSupported
>> class. e.g.
>>
>> FeatureSupported.networkLinking(getVm().getVdsGroupCompatibilityVersion())
>>
>> Checks whether the "network linking" feature is supported for the the
>> VM's cluster compatibility version. This internally checks whether
>> the
>> value of the corresponding config (NetworkLinkingSupported) for the
>> given compatibility version is true/false.
>>
>> I'm not sure if this is a good idea, since a feature is typically
>> supported "from" a particular version. E.g. Gluster support was
>> introduced in 3.1, and it continues to be available in all subsequent
>> versions. So I see no point in adding configuration for every version
>> indicating whether the feature is supported in that version or not. I
>> suggest to use either of the following options:
>>
>> 1) Instead of using a boolean config for each version, use a single
>> string config that indicates the "supported from" version e.g.
>> GlusterSupportedFrom = 3.1. There could be rare  cases where a
>> feature,
>> for some reason, is removed in some release. In such cases, we could
>> use
>> one additional config for the "supported to" version.
>>
>> 2) Continue with the boolean approach, but do not have entries for
>> every
>> version; rather make use of the "default value" for majority of
>> cases,
>> and add the explicit version mapping for the minority e.g.
>> GlusterSupported = true by default, and false in case of 3.0 (only
>> one
>> config required for 3.0)
>>
> i like this approach better,
> if one add new feature for 3.3 he should add it as 'true' in the config to be used as default,
> and explicitly add it as false for other unsupported versions.

For the record, this was the approach that was finally approved on the 
patch and merged (http://gerrit.ovirt.org/12970)
So I think now onwards, everyone can start using this approach for new 
features being added.

> if we do go this way, we need to make sure it works because i vaguely remember we had a bug in configuration,
> making us explicitly specify value for all versions.

I wrote a test case on DBConfigUtils that verifies that it does indeed 
work fine (http://gerrit.ovirt.org/13787), which has also been approved 
and merged.

>
>> Thoughts?
>>
>> Regards,
>> Shireesh
>> _______________________________________________
>> Engine-devel mailing list
>> Engine-devel at ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/engine-devel
>>