1:28 a.m.
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)
Thoughts?
Regards,
Shireesh
2:20 a.m.
----- Original Message -----
From: "Shireesh Anjal" <sanjal(a)redhat.com>
To: engine-devel(a)ovirt.org
Sent: Monday, March 18, 2013 11:58: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 think we can slightly modify this approach to make it better. I will start with an
example.
Lets say a feature 'XyzFeature' is introduced in 3.2 and available till 3.5. And
in 3.6, 'XyzFeature' is removed because of something better is available.
We can have the following configuration.
('XyzFeatureSupported','true','3.2')
('XyzFeatureSupported','false','3.6')
And the above will be interpreted as 'XyzFeature' is supported from 3.2 onwards
and not-supported from 3.6 onwards.
Thanks,
Kanagaraj
Thoughts?
Regards,
Shireesh
_______________________________________________
Engine-devel mailing list
Engine-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
2:29 a.m.
----- Original Message -----
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:
You can "merge" the configs into a single config when older versions go out of
the supported versions for the system.
i.e. in 4.0 you can have upgrade script that merges all GlusterFeatureSupported to one
entry instead of several.
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'm not sure why we would want to complicate this simple mechanism?
Is there much to gain?
Thoughts?
Regards,
Shireesh
_______________________________________________
Engine-devel mailing list
Engine-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
2:41 a.m.
On 03/18/2013 12:59 PM, Mike Kolesnik wrote:
----- Original Message -----
> 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:
You can "merge" the configs into a single config when older versions go out of
the supported versions for the system.
i.e. in 4.0 you can have upgrade script that merges all GlusterFeatureSupported to one
entry instead of several.
> 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'm not sure why we would want to complicate this simple mechanism?
Is there much to gain?
I think option 1 suggested above is simpler - to implement as well as to
understand.
Let me give you an example of why I don't like current mechanism. I
introduce a version check for a feature that was introduced in 3.1. I'm
being asked now to add three entries in config
3.0 - false
3.1 - true
3.2 - true
It will also mean that when 3.3 goes out, someone has to make sure that
another entry is added for 3.3-true. I think it is not logical as well
as scalable if you have more versions. And it sounds far more complex
(to maintain) than just having
<Feature>SupportedFrom = 3.1
So I would like to know if there are any objections to my proposal. I
intend to use this for at least the gluster related features.
> Thoughts?
>
> Regards,
> Shireesh
> _______________________________________________
> Engine-devel mailing list
> Engine-devel(a)ovirt.org
> http://lists.ovirt.org/mailman/listinfo/engine-devel
>
2:59 a.m.
----- Original Message -----
From: "Shireesh Anjal" <sanjal(a)redhat.com>
To: "Mike Kolesnik" <mkolesni(a)redhat.com>
Cc: engine-devel(a)ovirt.org
Sent: Monday, March 18, 2013 9:41:33 AM
Subject: Re: [Engine-devel] FeatureSupported and compatibility versions
On 03/18/2013 12:59 PM, Mike Kolesnik wrote:
> ----- Original Message -----
>> 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:
> You can "merge" the configs into a single config when older
> versions go out of the supported versions for the system.
>
> i.e. in 4.0 you can have upgrade script that merges all
> GlusterFeatureSupported to one entry instead of several.
>
>> 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'm not sure why we would want to complicate this simple mechanism?
>
> Is there much to gain?
I think option 1 suggested above is simpler - to implement as well as
to
understand.
Let me give you an example of why I don't like current mechanism. I
introduce a version check for a feature that was introduced in 3.1.
I'm
being asked now to add three entries in config
3.0 - false
3.1 - true
3.2 - true
It will also mean that when 3.3 goes out, someone has to make sure
that
another entry is added for 3.3-true. I think it is not logical as
well
as scalable if you have more versions. And it sounds far more complex
(to maintain) than just having
I think we should look at it from two directions -
a. the Java API we provide - Config and Feature classes
b. The database.
I personally also had thoughts that the current mechanism is lacking, in sense of defining
"ranges of versions" for features.
We should have a concept of "starts with version" and "supported until
version".
I think the logic for that should be implemented at db and at the Config class.
I think the API provided by featureSupported should be kept.
<Feature>SupportedFrom = 3.1
So I would like to know if there are any objections to my proposal. I
intend to use this for at least the gluster related features.
>> Thoughts?
>>
>> Regards,
>> Shireesh
>> _______________________________________________
>> 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
9:47 a.m.
On 03/18/2013 01:11 PM, Shireesh Anjal wrote:
On 03/18/2013 12:59 PM, Mike Kolesnik wrote:
> ----- Original Message -----
>> 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:
> You can "merge" the configs into a single config when older versions
> go out of the supported versions for the system.
>
> i.e. in 4.0 you can have upgrade script that merges all
> GlusterFeatureSupported to one entry instead of several.
Why are we even storing this information in config? Is this something
that can be "configured" at customer site?
>
>> 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'm not sure why we would want to complicate this simple mechanism?
>
> Is there much to gain?
I think option 1 suggested above is simpler - to implement as well as
to understand.
Let me give you an example of why I don't like current mechanism. I
introduce a version check for a feature that was introduced in 3.1.
I'm being asked now to add three entries in config
3.0 - false
3.1 - true
3.2 - true
It will also mean that when 3.3 goes out, someone has to make sure
that another entry is added for 3.3-true. I think it is not logical as
well as scalable if you have more versions. And it sounds far more
complex (to maintain) than just having
<Feature>SupportedFrom = 3.1
So I would like to know if there are any objections to my proposal. I
intend to use this for at least the gluster related features.
>> Thoughts?
>>
>> Regards,
>> Shireesh
>> _______________________________________________
>> 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
9:50 a.m.
----- Original Message -----
From: "Shireesh Anjal" <sanjal(a)redhat.com>
To: "Mike Kolesnik" <mkolesni(a)redhat.com>
Cc: engine-devel(a)ovirt.org
Sent: Wednesday, March 20, 2013 4:47:08 PM
Subject: Re: [Engine-devel] FeatureSupported and compatibility versions
On 03/18/2013 01:11 PM, Shireesh Anjal wrote:
> On 03/18/2013 12:59 PM, Mike Kolesnik wrote:
>> ----- Original Message -----
>>> 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:
>> You can "merge" the configs into a single config when older
>> versions
>> go out of the supported versions for the system.
>>
>> i.e. in 4.0 you can have upgrade script that merges all
>> GlusterFeatureSupported to one entry instead of several.
Why are we even storing this information in config? Is this something
that can be "configured" at customer site?
As previously explained (but off list :) ) , Config gives you the ability to have a
cachable "map" of entry (i.e - "feature name") per version and value.
I guess it was convinient for the developers to use that.
I also mentioned that customers/oVirt users should config the entries of vdc_options using
engine-config tool only.
Not all entries are exposed via engine-config.properties (and no, not just "is
feature supported" entries are hidden).
>>
>>> 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'm not sure why we would want to complicate this simple
>> mechanism?
>>
>> Is there much to gain?
>
> I think option 1 suggested above is simpler - to implement as well
> as
> to understand.
>
> Let me give you an example of why I don't like current mechanism. I
> introduce a version check for a feature that was introduced in 3.1.
> I'm being asked now to add three entries in config
>
> 3.0 - false
> 3.1 - true
> 3.2 - true
>
> It will also mean that when 3.3 goes out, someone has to make sure
> that another entry is added for 3.3-true. I think it is not logical
> as
> well as scalable if you have more versions. And it sounds far more
> complex (to maintain) than just having
>
> <Feature>SupportedFrom = 3.1
>
> So I would like to know if there are any objections to my proposal.
> I
> intend to use this for at least the gluster related features.
>
>>> Thoughts?
>>>
>>> Regards,
>>> Shireesh
>>> _______________________________________________
>>> 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
_______________________________________________
Engine-devel mailing list
Engine-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
3:52 a.m.
On 03/20/2013 08:20 PM, Yair Zaslavsky wrote:
----- Original Message -----
> From: "Shireesh Anjal" <sanjal(a)redhat.com>
> To: "Mike Kolesnik" <mkolesni(a)redhat.com>
> Cc: engine-devel(a)ovirt.org
> Sent: Wednesday, March 20, 2013 4:47:08 PM
> Subject: Re: [Engine-devel] FeatureSupported and compatibility versions
>
> On 03/18/2013 01:11 PM, Shireesh Anjal wrote:
>> On 03/18/2013 12:59 PM, Mike Kolesnik wrote:
>>> ----- Original Message -----
>>>> 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:
>>> You can "merge" the configs into a single config when older
>>> versions
>>> go out of the supported versions for the system.
>>>
>>> i.e. in 4.0 you can have upgrade script that merges all
>>> GlusterFeatureSupported to one entry instead of several.
> Why are we even storing this information in config? Is this something
> that can be "configured" at customer site?
As previously explained (but off list :) ) , Config gives you the ability to have a
cachable "map" of entry (i.e - "feature name") per version and value.
I guess it was convinient for the developers to use that.
I also mentioned that customers/oVirt users should config the entries of vdc_options
using engine-config tool only.
Not all entries are exposed via engine-config.properties (and no, not just "is
feature supported" entries are hidden).
>>>> 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'm not sure why we would want to complicate this simple
>>> mechanism?
>>>
>>> Is there much to gain?
>> I think option 1 suggested above is simpler - to implement as well
>> as
>> to understand.
>>
>> Let me give you an example of why I don't like current mechanism. I
>> introduce a version check for a feature that was introduced in 3.1.
>> I'm being asked now to add three entries in config
>>
>> 3.0 - false
>> 3.1 - true
>> 3.2 - true
>>
>> It will also mean that when 3.3 goes out, someone has to make sure
>> that another entry is added for 3.3-true. I think it is not logical
>> as
>> well as scalable if you have more versions. And it sounds far more
>> complex (to maintain) than just having
>>
>> <Feature>SupportedFrom = 3.1
>>
>> So I would like to know if there are any objections to my proposal.
>> I
>> intend to use this for at least the gluster related features.
I've sent a patch (http://gerrit.ovirt.org/12970) with following changes:
1) Introduced CompatibilityUtils that provides utility methods for
checking if a given feature is supported in the config. One method to
check based on boolean values (as is being done today for virt
features), and nother to check based on a range (from, to) which I would
like to use for gluster features.
2) Renamed FeatureSupported to VirtFeatureSupported, and made it use the
first utility method from CompatibilityUtils
3) Introduced GlusterFeatureSupported for gluster features, which uses
the second utility method from CompatibilityUtils
Key advantage here is that
- we don't have to touch any virt specifc source for adding
compatibility checks for gluster features
- virt features continue to use the existing boolean config check
Any comments / suggestions / reviews will be highly appreciated :)
>>>> Thoughts?
>>>>
>>>> Regards,
>>>> Shireesh
>>>> _______________________________________________
>>>> 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
> _______________________________________________
> 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
7:18 a.m.
----- Original Message -----
On 03/20/2013 08:20 PM, Yair Zaslavsky wrote:
>
> ----- Original Message -----
>> From: "Shireesh Anjal" <sanjal(a)redhat.com>
>> To: "Mike Kolesnik" <mkolesni(a)redhat.com>
>> Cc: engine-devel(a)ovirt.org
>> Sent: Wednesday, March 20, 2013 4:47:08 PM
>> Subject: Re: [Engine-devel] FeatureSupported and compatibility
>> versions
>>
>> On 03/18/2013 01:11 PM, Shireesh Anjal wrote:
>>> On 03/18/2013 12:59 PM, Mike Kolesnik wrote:
>>>> ----- Original Message -----
>>>>> 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:
>>>> You can "merge" the configs into a single config when older
>>>> versions
>>>> go out of the supported versions for the system.
>>>>
>>>> i.e. in 4.0 you can have upgrade script that merges all
>>>> GlusterFeatureSupported to one entry instead of several.
>> Why are we even storing this information in config? Is this
>> something
>> that can be "configured" at customer site?
> As previously explained (but off list :) ) , Config gives you the
> ability to have a cachable "map" of entry (i.e - "feature
name")
> per version and value.
> I guess it was convinient for the developers to use that.
> I also mentioned that customers/oVirt users should config the
> entries of vdc_options using engine-config tool only.
> Not all entries are exposed via engine-config.properties (and no,
> not just "is feature supported" entries are hidden).
>
>
>
>
>>>>> 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'm not sure why we would want to complicate this simple
>>>> mechanism?
>>>>
>>>> Is there much to gain?
>>> I think option 1 suggested above is simpler - to implement as
>>> well
>>> as
>>> to understand.
>>>
>>> Let me give you an example of why I don't like current mechanism.
>>> I
>>> introduce a version check for a feature that was introduced in
>>> 3.1.
>>> I'm being asked now to add three entries in config
>>>
>>> 3.0 - false
>>> 3.1 - true
>>> 3.2 - true
>>>
>>> It will also mean that when 3.3 goes out, someone has to make
>>> sure
>>> that another entry is added for 3.3-true. I think it is not
>>> logical
>>> as
>>> well as scalable if you have more versions. And it sounds far
>>> more
>>> complex (to maintain) than just having
>>>
>>> <Feature>SupportedFrom = 3.1
>>>
>>> So I would like to know if there are any objections to my
>>> proposal.
>>> I
>>> intend to use this for at least the gluster related features.
I've sent a patch (http://gerrit.ovirt.org/12970) with following
changes:
1) Introduced CompatibilityUtils that provides utility methods for
checking if a given feature is supported in the config. One method to
check based on boolean values (as is being done today for virt
features), and nother to check based on a range (from, to) which I
would
like to use for gluster features.
2) Renamed FeatureSupported to VirtFeatureSupported, and made it use
the
first utility method from CompatibilityUtils
3) Introduced GlusterFeatureSupported for gluster features, which
uses
the second utility method from CompatibilityUtils
Key advantage here is that
- we don't have to touch any virt specifc source for adding
compatibility checks for gluster features
- virt features continue to use the existing boolean config check
Any comments / suggestions / reviews will be highly appreciated :)
I think splitting to two classes is OK, but the underlying mechanism IMO should be as Omer
suggested:
Use the default value from the java config file, and if in the DB there is a version
specific value then use it for that version only.
I don't think "From, To, etc" is a good design, it's not a standard way
and is very restrictive.
>>>>> Thoughts?
>>>>>
>>>>> Regards,
>>>>> Shireesh
7:47 a.m.
This is a multi-part message in MIME format.
--------------020009000007090902020606
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
On 03/27/2013 05:48 PM, Mike Kolesnik wrote:
----- Original Message -----
> On 03/20/2013 08:20 PM, Yair Zaslavsky wrote:
>> ----- Original Message -----
>>> From: "Shireesh Anjal" <sanjal(a)redhat.com>
>>> To: "Mike Kolesnik" <mkolesni(a)redhat.com>
>>> Cc: engine-devel(a)ovirt.org
>>> Sent: Wednesday, March 20, 2013 4:47:08 PM
>>> Subject: Re: [Engine-devel] FeatureSupported and compatibility
>>> versions
>>>
>>> On 03/18/2013 01:11 PM, Shireesh Anjal wrote:
>>>> On 03/18/2013 12:59 PM, Mike Kolesnik wrote:
>>>>> ----- Original Message -----
>>>>>> 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:
>>>>> You can "merge" the configs into a single config when
older
>>>>> versions
>>>>> go out of the supported versions for the system.
>>>>>
>>>>> i.e. in 4.0 you can have upgrade script that merges all
>>>>> GlusterFeatureSupported to one entry instead of several.
>>> Why are we even storing this information in config? Is this
>>> something
>>> that can be "configured" at customer site?
>> As previously explained (but off list :) ) , Config gives you the
>> ability to have a cachable "map" of entry (i.e - "feature
name")
>> per version and value.
>> I guess it was convinient for the developers to use that.
>> I also mentioned that customers/oVirt users should config the
>> entries of vdc_options using engine-config tool only.
>> Not all entries are exposed via engine-config.properties (and no,
>> not just "is feature supported" entries are hidden).
>>
>>
>>
>>
>>>>>> 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'm not sure why we would want to complicate this simple
>>>>> mechanism?
>>>>>
>>>>> Is there much to gain?
>>>> I think option 1 suggested above is simpler - to implement as
>>>> well
>>>> as
>>>> to understand.
>>>>
>>>> Let me give you an example of why I don't like current mechanism.
>>>> I
>>>> introduce a version check for a feature that was introduced in
>>>> 3.1.
>>>> I'm being asked now to add three entries in config
>>>>
>>>> 3.0 - false
>>>> 3.1 - true
>>>> 3.2 - true
>>>>
>>>> It will also mean that when 3.3 goes out, someone has to make
>>>> sure
>>>> that another entry is added for 3.3-true. I think it is not
>>>> logical
>>>> as
>>>> well as scalable if you have more versions. And it sounds far
>>>> more
>>>> complex (to maintain) than just having
>>>>
>>>> <Feature>SupportedFrom = 3.1
>>>>
>>>> So I would like to know if there are any objections to my
>>>> proposal.
>>>> I
>>>> intend to use this for at least the gluster related features.
> I've sent a patch (http://gerrit.ovirt.org/12970) with following
> changes:
>
> 1) Introduced CompatibilityUtils that provides utility methods for
> checking if a given feature is supported in the config. One method to
> check based on boolean values (as is being done today for virt
> features), and nother to check based on a range (from, to) which I
> would
> like to use for gluster features.
> 2) Renamed FeatureSupported to VirtFeatureSupported, and made it use
> the
> first utility method from CompatibilityUtils
> 3) Introduced GlusterFeatureSupported for gluster features, which
> uses
> the second utility method from CompatibilityUtils
>
> Key advantage here is that
> - we don't have to touch any virt specifc source for adding
> compatibility checks for gluster features
> - virt features continue to use the existing boolean config check
>
> Any comments / suggestions / reviews will be highly appreciated :)
I think splitting to two classes is OK, but the underlying mechanism IMO should be as
Omer suggested:
Use the default value from the java config file, and if in the DB there is a version
specific value then use it for that version only.
Review comments here are on the contrary:
http://gerrit.ovirt.org/#/c/12970/5/backend/manager/dbscripts/upgrade/pre...
I don't think "From, To, etc" is a good design,
it's not a standard way and is very restrictive.
Can you please explain in what way is it restrictive?
Also, what is the "etc" you are referring to?
>>>>>> Thoughts?
>>>>>>
>>>>>> Regards,
>>>>>> Shireesh
--------------020009000007090902020606
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: 7bit
<html>
<head>
<meta content="text/html; charset=UTF-8"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">On 03/27/2013 05:48 PM, Mike Kolesnik
wrote:<br>
</div>
<blockquote
cite="mid:36623975.7044697.1364386718971.JavaMail.root@redhat.com"
type="cite">
<pre wrap="">----- Original Message -----
</pre>
<blockquote type="cite">
<pre wrap="">On 03/20/2013 08:20 PM, Yair Zaslavsky wrote:
</pre>
<blockquote type="cite">
<pre wrap="">
----- Original Message -----
</pre>
<blockquote type="cite">
<pre wrap="">From: "Shireesh Anjal" <a
class="moz-txt-link-rfc2396E"
href="mailto:sanjal@redhat.com"><sanjal@redhat.com></a>
To: "Mike Kolesnik" <a class="moz-txt-link-rfc2396E"
href="mailto:mkolesni@redhat.com"><mkolesni@redhat.com></a>
Cc: <a class="moz-txt-link-abbreviated"
href="mailto:engine-devel@ovirt.org">engine-devel@ovirt.org</a>
Sent: Wednesday, March 20, 2013 4:47:08 PM
Subject: Re: [Engine-devel] FeatureSupported and compatibility
versions
On 03/18/2013 01:11 PM, Shireesh Anjal wrote:
</pre>
<blockquote type="cite">
<pre wrap="">On 03/18/2013 12:59 PM, Mike Kolesnik wrote:
</pre>
<blockquote type="cite">
<pre wrap="">----- Original Message -----
</pre>
<blockquote type="cite">
<pre wrap="">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:
</pre>
</blockquote>
<pre wrap="">You can "merge" the configs into a
single config when older
versions
go out of the supported versions for the system.
i.e. in 4.0 you can have upgrade script that merges all
GlusterFeatureSupported to one entry instead of several.
</pre>
</blockquote>
</blockquote>
<pre wrap="">Why are we even storing this information in
config? Is this
something
that can be "configured" at customer site?
</pre>
</blockquote>
<pre wrap="">As previously explained (but off list :) ) , Config
gives you the
ability to have a cachable "map" of entry (i.e - "feature name")
per version and value.
I guess it was convinient for the developers to use that.
I also mentioned that customers/oVirt users should config the
entries of vdc_options using engine-config tool only.
Not all entries are exposed via engine-config.properties (and no,
not just "is feature supported" entries are hidden).
</pre>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<pre wrap="">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)
</pre>
</blockquote>
<pre wrap="">I'm not sure why we would want to
complicate this simple
mechanism?
Is there much to gain?
</pre>
</blockquote>
<pre wrap="">I think option 1 suggested above is simpler -
to implement as
well
as
to understand.
Let me give you an example of why I don't like current mechanism.
I
introduce a version check for a feature that was introduced in
3.1.
I'm being asked now to add three entries in config
3.0 - false
3.1 - true
3.2 - true
It will also mean that when 3.3 goes out, someone has to make
sure
that another entry is added for 3.3-true. I think it is not
logical
as
well as scalable if you have more versions. And it sounds far
more
complex (to maintain) than just having
<Feature>SupportedFrom = 3.1
So I would like to know if there are any objections to my
proposal.
I
intend to use this for at least the gluster related features.
</pre>
</blockquote>
</blockquote>
</blockquote>
<pre wrap="">
I've sent a patch (<a class="moz-txt-link-freetext"
href="http://gerrit.ovirt.org/12970">http://gerrit.ovirt.org...>)
with following
changes:
1) Introduced CompatibilityUtils that provides utility methods for
checking if a given feature is supported in the config. One method to
check based on boolean values (as is being done today for virt
features), and nother to check based on a range (from, to) which I
would
like to use for gluster features.
2) Renamed FeatureSupported to VirtFeatureSupported, and made it use
the
first utility method from CompatibilityUtils
3) Introduced GlusterFeatureSupported for gluster features, which
uses
the second utility method from CompatibilityUtils
Key advantage here is that
- we don't have to touch any virt specifc source for adding
compatibility checks for gluster features
- virt features continue to use the existing boolean config check
Any comments / suggestions / reviews will be highly appreciated :)
</pre>
</blockquote>
<pre wrap="">
I think splitting to two classes is OK, but the underlying mechanism IMO should be as Omer
suggested:
Use the default value from the java config file, and if in the DB there is a version
specific value then use it for that version only.</pre>
</blockquote>
<br>
Review comments here are on the contrary:<br>
<meta http-equiv="content-type" content="text/html;
charset=UTF-8">
<a
href="http://gerrit.ovirt.org/#/c/12970/5/backend/manager/dbscripts/...
<br>
<blockquote
cite="mid:36623975.7044697.1364386718971.JavaMail.root@redhat.com"
type="cite">
<pre wrap="">
I don't think "From, To, etc" is a good design, it's not a standard way
and is very restrictive.</pre>
</blockquote>
<br>
Can you please explain in what way is it restrictive?<br>
<br>
Also, what is the "etc" you are referring to?<br>
<br>
<blockquote
cite="mid:36623975.7044697.1364386718971.JavaMail.root@redhat.com"
type="cite">
<pre wrap="">
</pre>
<blockquote type="cite">
<pre wrap="">
</pre>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<pre wrap="">Thoughts?
Regards,
Shireesh
</pre>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
<br>
</body>
</html>
--------------020009000007090902020606--
4:17 a.m.
------=_Part_574909_671557636.1364894251175
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
----- Original Message -----
On 03/27/2013 05:48 PM, Mike Kolesnik wrote:
> ----- Original Message -----
> > On 03/20/2013 08:20 PM, Yair Zaslavsky wrote:
>
> > > ----- Original Message -----
> >
>
> > > > From: "Shireesh Anjal"
<sanjal(a)redhat.com> To: "Mike Kolesnik"
> > > > <mkolesni(a)redhat.com> Cc: engine-devel(a)ovirt.org Sent:
Wednesday,
> > > > March
> > > > 20,
> > > > 2013 4:47:08 PM
> > >
> >
>
> > > > Subject: Re: [Engine-devel] FeatureSupported and compatibility
> > >
> >
>
> > > > versions
> > >
> >
>
> > > > On 03/18/2013 01:11 PM, Shireesh Anjal wrote:
> > >
> >
>
> > > > > On 03/18/2013 12:59 PM, Mike Kolesnik
wrote:
> > > >
> > >
> >
>
> > > > > > ----- Original Message -----
> > > > >
> > > >
> > >
> >
>
> > > > > > > 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:
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > You can "merge" the configs
into a single config when older
> > > > >
> > > >
> > >
> >
>
> > > > > > versions
> > > > >
> > > >
> > >
> >
>
> > > > > > go out of the supported versions for the system.
> > > > >
> > > >
> > >
> >
>
> > > > > > i.e. in 4.0 you can have upgrade script
that merges all
> > > > >
> > > >
> > >
> >
>
> > > > > > GlusterFeatureSupported to one entry instead of several.
> > > > >
> > > >
> > >
> >
>
> > > > Why are we even storing this information in
config? Is this
> > >
> >
>
> > > > something
> > >
> >
>
> > > > that can be "configured" at customer site?
> > >
> >
>
> > > As previously explained (but off list :) ) , Config
gives you the
> >
>
> > > ability to have a cachable "map" of entry (i.e - "feature
name")
> >
>
> > > per version and value.
> >
>
> > > I guess it was convinient for the developers to use that.
> >
>
> > > I also mentioned that customers/oVirt users should config the
> >
>
> > > entries of vdc_options using engine-config tool only.
> >
>
> > > Not all entries are exposed via engine-config.properties (and no,
> >
>
> > > not just "is feature supported" entries are hidden).
> >
>
> > > > > > > 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'm not sure why we would want to
complicate this simple
> > > > >
> > > >
> > >
> >
>
> > > > > > mechanism?
> > > > >
> > > >
> > >
> >
>
> > > > > > Is there much to gain?
> > > > >
> > > >
> > >
> >
>
> > > > > I think option 1 suggested above is simpler
- to implement as
> > > >
> > >
> >
>
> > > > > well
> > > >
> > >
> >
>
> > > > > as
> > > >
> > >
> >
>
> > > > > to understand.
> > > >
> > >
> >
>
> > > > > Let me give you an example of why I
don't like current mechanism.
> > > >
> > >
> >
>
> > > > > I
> > > >
> > >
> >
>
> > > > > introduce a version check for a feature that was introduced in
> > > >
> > >
> >
>
> > > > > 3.1.
> > > >
> > >
> >
>
> > > > > I'm being asked now to add three entries in config
> > > >
> > >
> >
>
> > > > > 3.0 - false
> > > >
> > >
> >
>
> > > > > 3.1 - true
> > > >
> > >
> >
>
> > > > > 3.2 - true
> > > >
> > >
> >
>
> > > > > It will also mean that when 3.3 goes out,
someone has to make
> > > >
> > >
> >
>
> > > > > sure
> > > >
> > >
> >
>
> > > > > that another entry is added for 3.3-true. I think it is not
> > > >
> > >
> >
>
> > > > > logical
> > > >
> > >
> >
>
> > > > > as
> > > >
> > >
> >
>
> > > > > well as scalable if you have more versions. And it sounds far
> > > >
> > >
> >
>
> > > > > more
> > > >
> > >
> >
>
> > > > > complex (to maintain) than just having
> > > >
> > >
> >
>
> > > > > <Feature>SupportedFrom = 3.1
> > > >
> > >
> >
>
> > > > > So I would like to know if there are any
objections to my
> > > >
> > >
> >
>
> > > > > proposal.
> > > >
> > >
> >
>
> > > > > I
> > > >
> > >
> >
>
> > > > > intend to use this for at least the gluster related features.
> > > >
> > >
> >
>
> > I've sent a patch ( http://gerrit.ovirt.org/12970 )
with following
>
> > changes:
>
> > 1) Introduced CompatibilityUtils that provides utility
methods for
>
> > checking if a given feature is supported in the config. One method to
>
> > check based on boolean values (as is being done today for virt
>
> > features), and nother to check based on a range (from, to) which I
>
> > would
>
> > like to use for gluster features.
>
> > 2) Renamed FeatureSupported to VirtFeatureSupported, and made it use
>
> > the
>
> > first utility method from CompatibilityUtils
>
> > 3) Introduced GlusterFeatureSupported for gluster features, which
>
> > uses
>
> > the second utility method from CompatibilityUtils
>
> > Key advantage here is that
>
> > - we don't have to touch any virt specifc source for adding
>
> > compatibility checks for gluster features
>
> > - virt features continue to use the existing boolean config check
>
> > Any comments / suggestions / reviews will be highly
appreciated :)
>
> I think splitting to two classes is OK, but the underlying
mechanism IMO
> should be as Omer suggested:
> Use the default value from the java config file, and if in the DB there is
> a
> version specific value then use it for that version only.
Review comments here are on the contrary:
http://gerrit.ovirt.org/#/c/12970/5/backend/manager/dbscripts/upgrade/pre...
The comment in the review simply states that the mechanism is probably broken, not that
that's the way it has to be done.
> I don't think "From, To, etc" is a good design,
it's not a standard way and
> is very restrictive.
Can you please explain in what way is it restrictive?
Also, what is the "etc" you are referring to?
What if
for certain version it is not supported, you add "except"? Or do you specify 2
ranges?
Starting to add from/to creates a limited design of one range, which would be difficult to
tune if necessary.
I think the design generally for config values is very simple and suits us well - use the
default value, unless a specific version is configured differently.
This way you can specify the feature is supported, and disable it for specific versions.
I think this direction gives us the flexibility that we would like to have.
Currently it doesn't work that way, but I think it's not impossible to change, and
more worthwhile than introducing a new mechanism.
> > > > > > > Thoughts?
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > Regards,
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > Shireesh
> > > > > >
> > > > >
> > > >
> > >
> >
>
------=_Part_574909_671557636.1364894251175
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
class=3D"moz-cite-prefix">On 03/2=
7/2013 05:48 PM, Mike Kolesnik wrote:<br></div><blockquote
cite=3D"mid:3662=
3975.7044697.1364386718971.JavaMail.root(a)redhat.com"><pre>----- Original
Me=
ssage -----
</pre><blockquote><pre>On 03/20/2013 08:20 PM, Yair Zaslavsky wrote:
</pre><blockquote><pre>----- Original Message -----
</pre><blockquote><pre>From: "Shireesh Anjal" <a
class=3D"moz-txt-link-rfc2=
396E" href=3D"mailto:sanjal@redhat.com" target=3D"_blank"
data-mce-href=3D"=
mailto:sanjal@redhat.com"><sanjal@redhat.com></a>
To: "Mike Kolesnik" <a class=3D"moz-txt-link-rfc2396E"
href=3D"mailto:mkole=
sni(a)redhat.com" target=3D"_blank"
data-mce-href=3D"mailto:mkolesni@redhat.c=
om">&lt;mkolesni(a)redhat.com&gt;</a>
Cc: <a class=3D"moz-txt-link-abbreviated"
href=3D"mailto:engine-devel@ovirt=
.org" target=3D"_blank"
data-mce-href=3D"mailto:engine-devel@ovirt.org">eng=
ine-devel(a)ovirt.org</a>
Sent: Wednesday, March 20, 2013 4:47:08 PM
Subject: Re: [Engine-devel] FeatureSupported and compatibility
versions
On 03/18/2013 01:11 PM, Shireesh Anjal wrote:
</pre><blockquote><pre>On 03/18/2013 12:59 PM, Mike Kolesnik wrote:
</pre><blockquote><pre>----- Original Message -----
</pre><blockquote><pre>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:
</pre></blockquote><pre>You can "merge" the configs into a
single config wh=
en older
versions
go out of the supported versions for the system.
i.e. in 4.0 you can have upgrade script that merges all
GlusterFeatureSupported to one entry instead of several.
</pre></blockquote></blockquote><pre>Why are we even storing this
informati=
on in config? Is this
something
that can be "configured" at customer site?
</pre></blockquote><pre>As previously explained (but off list :) ) ,
Config=
gives you the
ability to have a cachable "map" of entry (i.e - "feature name")
per version and value.
I guess it was convinient for the developers to use that.
I also mentioned that customers/oVirt users should config the
entries of vdc_options using engine-config tool only.
Not all entries are exposed via engine-config.properties (and no,
not just "is feature supported" entries are hidden).
</pre><blockquote><blockquote><blockquote><blockquote><pre>1)
Instead of us=
ing a boolean config for each version, use a
single
string config that indicates the "supported from" version e.g.
GlusterSupportedFrom =3D 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 =3D true by default, and false in case of 3.0
(only
one
config required for 3.0)
</pre></blockquote><pre>I'm not sure why we would want to complicate
this s=
imple
mechanism?
Is there much to gain?
</pre></blockquote><pre>I think option 1 suggested above is simpler - to
im=
plement as
well
as
to understand.
Let me give you an example of why I don't like current mechanism.
I
introduce a version check for a feature that was introduced in
3.1.
I'm being asked now to add three entries in config
3.0 - false
3.1 - true
3.2 - true
It will also mean that when 3.3 goes out, someone has to make
sure
that another entry is added for 3.3-true. I think it is not
logical
as
well as scalable if you have more versions. And it sounds far
more
complex (to maintain) than just having
<Feature>SupportedFrom =3D 3.1
So I would like to know if there are any objections to my
proposal.
I
intend to use this for at least the gluster related features.
</pre></blockquote></blockquote></blockquote><pre>I've
sent a patch (<a cla=
ss=3D"moz-txt-link-freetext" href=3D"http://gerrit.ovirt.org/12970"
target=
=3D"_blank"
data-mce-href=3D"http://gerrit.ovirt.org/12970">http://gerri...
virt.org/12970</a>) with following
changes:
1) Introduced CompatibilityUtils that provides utility methods for
checking if a given feature is supported in the config. One method to
check based on boolean values (as is being done today for virt
features), and nother to check based on a range (from, to) which I
would
like to use for gluster features.
2) Renamed FeatureSupported to VirtFeatureSupported, and made it use
the
first utility method from CompatibilityUtils
3) Introduced GlusterFeatureSupported for gluster features, which
uses
the second utility method from CompatibilityUtils
Key advantage here is that
- we don't have to touch any virt specifc source for adding
compatibility checks for gluster features
- virt features continue to use the existing boolean config check
Any comments / suggestions / reviews will be highly appreciated :)
</pre></blockquote><pre>I think splitting to two classes is OK, but the
und=
erlying mechanism IMO should be as Omer suggested:
Use the default value from the java config file, and if in the DB there is =
a version specific value then use it for that version only.</pre></blockquo=
te><br> Review comments here are on the contrary:<br><a
href=3D"http://gerr=
it.ovirt.org/#/c/12970/5/backend/manager/dbscripts/upgrade/pre_upgrade/0000=
_config.sql" target=3D"_blank"
data-mce-href=3D"http://gerrit.ovirt.org/#/c=
/12970/5/backend/manager/dbscripts/upgrade/pre_upgrade/0000_config.sql">htt=
p://gerrit.ovirt.org/#/c/12970/5/backend/manager/dbscripts/upgrade/pre_upgr=
ade/0000_config.sql</a><br
data-mce-bogus=3D"1"></blockquote><div>The comme=
nt in the review simply states that the mechanism is probably broken, not t=
hat that's the way it has to be done.<br></div><blockquote
style=3D"border-=
left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-wei=
ght:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Ari=
al,sans-serif;font-size:12pt;" data-mce-style=3D"border-left: 2px solid #10=
10FF; margin-left: 5px; padding-left: 5px; color: #000; font-weight: normal=
; font-style: normal; text-decoration: none; font-family: Helvetica,Arial,s=
ans-serif; font-size: 12pt;"><br><blockquote
cite=3D"mid:36623975.7044697.1=
364386718971.JavaMail.root(a)redhat.com"><pre>I don't think "From,
To, etc" i=
s a good design, it's not a standard way and is very restrictive.</pre></bl=
ockquote><br> Can you please explain in what way is it
restrictive?<br><br>=
Also, what is the "etc" you are referring to?</blockquote><div>What
if for=
certain version it is not supported, you add "except"? Or do you specify 2=
ranges?<br></div><div><br></div><div>Starting to add
from/to creates a lim=
ited design of one range, which would be difficult to tune if necessary.</d=
iv><div><br></div><div>I think the design generally for config
values is ve=
ry simple and suits us well - use the default value, unless a specific vers=
ion is configured differently.<br></div><div>This way you can specify
the f=
eature is supported, and disable it for specific
versions.<br></div><div>I =
think this direction gives us the flexibility that we would like to have.<b=
r></div><div><br></div><div>Currently it doesn't work
that way, but I think=
it's not impossible to change, and more worthwhile than introducing a new =
mechanism.<br></div><blockquote style=3D"border-left:2px solid
#1010FF;marg=
in-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:norma=
l;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12p=
t;" data-mce-style=3D"border-left: 2px solid #1010FF; margin-left: 5px; pad=
ding-left: 5px; color: #000; font-weight: normal; font-style: normal; text-=
decoration: none; font-family: Helvetica,Arial,sans-serif; font-size: 12pt;=
"><br><blockquote
cite=3D"mid:36623975.7044697.1364386718971.JavaMail.root@=
redhat.com"><blockquote><blockquote><blockquote><blockquote><blockquote><bl=
ockquote><pre>Thoughts?
Regards,
Shireesh
</pre></blockquote></blockquote></blockquote></blockquote></blockquote></bl=
ockquote></blockquote><br></blockquote><div><br></div></div></body></html>
------=_Part_574909_671557636.1364894251175--
6 a.m.
This is a multi-part message in MIME format.
--------------060100050808070407030307
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
On 04/02/2013 02:47 PM, Mike Kolesnik wrote:
------------------------------------------------------------------------
On 03/27/2013 05:48 PM, Mike Kolesnik wrote:
----- Original Message -----
On 03/20/2013 08:20 PM, Yair Zaslavsky wrote:
----- Original Message -----
From: "Shireesh Anjal"<sanjal(a)redhat.com>
To: "Mike Kolesnik"<mkolesni(a)redhat.com>
Cc:engine-devel@ovirt.org
Sent: Wednesday, March 20, 2013 4:47:08 PM
Subject: Re: [Engine-devel] FeatureSupported and compatibility
versions
On 03/18/2013 01:11 PM, Shireesh Anjal wrote:
On 03/18/2013 12:59 PM, Mike Kolesnik wrote:
----- Original Message -----
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:
You can "merge" the configs into a single config
when older
versions
go out of the supported versions for the system.
i.e. in 4.0 you can have upgrade script that merges all
GlusterFeatureSupported to one entry instead of several.
Why are we even storing this information in config? Is this
something
that can be "configured" at customer site?
As previously explained (but off list :) ) , Config gives you the
ability to have a cachable "map" of entry (i.e - "feature
name")
per version and value.
I guess it was convinient for the developers to use that.
I also mentioned that customers/oVirt users should config the
entries of vdc_options using engine-config tool only.
Not all entries are exposed via engine-config.properties (and no,
not just "is feature supported" entries are hidden).
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'm not sure why we would want to complicate this simple
mechanism?
Is there much to gain?
I think option 1 suggested above is simpler - to implement as
well
as
to understand.
Let me give you an example of why I don't like current
mechanism.
I
introduce a version check for a feature that was introduced in
3.1.
I'm being asked now to add three entries in config
3.0 - false
3.1 - true
3.2 - true
It will also mean that when 3.3 goes out, someone has to make
sure
that another entry is added for 3.3-true. I think it is not
logical
as
well as scalable if you have more versions. And it sounds far
more
complex (to maintain) than just having
<Feature>SupportedFrom = 3.1
So I would like to know if there are any objections to my
proposal.
I
intend to use this for at least the gluster related features.
I've sent a patch (http://gerrit.ovirt.org/12970) with following
changes:
1) Introduced CompatibilityUtils that provides utility methods for
checking if a given feature is supported in the config. One method to
check based on boolean values (as is being done today for virt
features), and nother to check based on a range (from, to) which I
would
like to use for gluster features.
2) Renamed FeatureSupported to VirtFeatureSupported, and made it use
the
first utility method from CompatibilityUtils
3) Introduced GlusterFeatureSupported for gluster features, which
uses
the second utility method from CompatibilityUtils
Key advantage here is that
- we don't have to touch any virt specifc source for adding
compatibility checks for gluster features
- virt features continue to use the existing boolean config check
Any comments / suggestions / reviews will be highly appreciated :)
I think splitting to two classes is OK, but the underlying mechanism IMO should
be as Omer suggested:
Use the default value from the java config file, and if in the DB there is a
version specific value then use it for that version only.
Review comments here are on the contrary:
http://gerrit.ovirt.org/#/c/12970/5/backend/manager/dbscripts/upgrade/pre...
The comment in the review simply states that the mechanism is probably
broken, not that that's the way it has to be done.
The comment explicitly asks me to add entries for every version. What
you have looked at is my response to this comment, which suggests that
the current mechanism is not great. In fact, what I had done in
patch-set 5 is exactly what you are suggesting : true as default value
and explicit entries in config for the "false" values. But it was not
accepted.
I don't think "From, To, etc" is a good design, it's not a
standard way and is very restrictive.
Can you please explain in what way is it restrictive?
Also, what is the "etc" you are referring to?
What if for certain version it is not supported, you add "except"? Or
do you specify 2 ranges?
Starting to add from/to creates a limited design of one range, which
would be difficult to tune if necessary.
Really? Does someone really think that there will be a feature that will
be supported in multiple different ranges of versions? I see zero
possibility for this. I would love to see some +1s on this concept
before I can accept this argument.
I think the design generally for config values is very simple and
suits us well - use the default value, unless a specific version is
configured differently.
I think the current design is wrong. A feature gets supported "from" a
particular version, and that's all that is required in most of the
cases. Expecting developers to add version-by-version mapping for
features is bad. The "to" part in my patch is just to handle rare cases,
if at all they come up. I'm willing to even remove that if such a case
doesn't exist today.
Also, even though I have followed it for the sake of consistency, I
don't think these values need to be stored in the config (db) at all.
Only explanation I've got for it is that it was probably 'convenient'
for developers to use the config mechanism. I'm for having this check
purely in code in a central place, and not the config (db).
This way you can specify the feature is supported, and disable it for
specific versions.
So one has to look at both code (FeatureSupported) as well as db
(config) to get an idea of what versions the feature is supported in.
Not great.
I think this direction gives us the flexibility that we would like to
have.
Currently it doesn't work that way, but I think it's not impossible to
change, and more worthwhile than introducing a new mechanism.
I disagree, and would like to use the "supported from" mechanism at
least for gluster features.
Thoughts?
Regards,
Shireesh
--------------060100050808070407030307
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: 7bit
<html>
<head>
<meta content="text/html; charset=UTF-8"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">On 04/02/2013 02:47 PM, Mike Kolesnik
wrote:<br>
</div>
<blockquote
cite="mid:1842623267.574910.1364894251176.JavaMail.root@redhat.com"
type="cite">
<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;"
data-mce-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 class="moz-cite-prefix">On 03/27/2013 05:48 PM, Mike
Kolesnik wrote:<br>
</div>
<blockquote
cite="mid:36623975.7044697.1364386718971.JavaMail.root@redhat.com">
<pre>----- Original Message -----
</pre>
<blockquote>
<pre>On 03/20/2013 08:20 PM, Yair Zaslavsky wrote:
</pre>
<blockquote>
<pre>----- Original Message -----
</pre>
<blockquote>
<pre>From: "Shireesh Anjal" <a
moz-do-not-send="true" class="moz-txt-link-rfc2396E"
href="mailto:sanjal@redhat.com" target="_blank"
data-mce-href="mailto:sanjal@redhat.com"><sanjal@redhat.com></a>
To: "Mike Kolesnik" <a moz-do-not-send="true"
class="moz-txt-link-rfc2396E" href="mailto:mkolesni@redhat.com"
target="_blank"
data-mce-href="mailto:mkolesni@redhat.com"><mkolesni@redhat.com></a>
Cc: <a moz-do-not-send="true" class="moz-txt-link-abbreviated"
href="mailto:engine-devel@ovirt.org" target="_blank"
data-mce-href="mailto:engine-devel@ovirt.org">engine-devel@ovirt.org</a>
Sent: Wednesday, March 20, 2013 4:47:08 PM
Subject: Re: [Engine-devel] FeatureSupported and compatibility
versions
On 03/18/2013 01:11 PM, Shireesh Anjal wrote:
</pre>
<blockquote>
<pre>On 03/18/2013 12:59 PM, Mike Kolesnik wrote:
</pre>
<blockquote>
<pre>----- Original Message -----
</pre>
<blockquote>
<pre>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:
</pre>
</blockquote>
<pre>You can "merge" the configs into a single
config when older
versions
go out of the supported versions for the system.
i.e. in 4.0 you can have upgrade script that merges all
GlusterFeatureSupported to one entry instead of several.
</pre>
</blockquote>
</blockquote>
<pre>Why are we even storing this information in config? Is this
something
that can be "configured" at customer site?
</pre>
</blockquote>
<pre>As previously explained (but off list :) ) , Config gives you
the
ability to have a cachable "map" of entry (i.e - "feature name")
per version and value.
I guess it was convinient for the developers to use that.
I also mentioned that customers/oVirt users should config the
entries of vdc_options using engine-config tool only.
Not all entries are exposed via engine-config.properties (and no,
not just "is feature supported" entries are hidden).
</pre>
<blockquote>
<blockquote>
<blockquote>
<blockquote>
<pre>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)
</pre>
</blockquote>
<pre>I'm not sure why we would want to complicate this
simple
mechanism?
Is there much to gain?
</pre>
</blockquote>
<pre>I think option 1 suggested above is simpler - to implement
as
well
as
to understand.
Let me give you an example of why I don't like current mechanism.
I
introduce a version check for a feature that was introduced in
3.1.
I'm being asked now to add three entries in config
3.0 - false
3.1 - true
3.2 - true
It will also mean that when 3.3 goes out, someone has to make
sure
that another entry is added for 3.3-true. I think it is not
logical
as
well as scalable if you have more versions. And it sounds far
more
complex (to maintain) than just having
<Feature>SupportedFrom = 3.1
So I would like to know if there are any objections to my
proposal.
I
intend to use this for at least the gluster related features.
</pre>
</blockquote>
</blockquote>
</blockquote>
<pre>I've sent a patch (<a moz-do-not-send="true"
class="moz-txt-link-freetext" href="http://gerrit.ovirt.org/12970"
target="_blank"
data-mce-href="http://gerrit.ovirt.org/12970">http://gerrit....>)
with following
changes:
1) Introduced CompatibilityUtils that provides utility methods for
checking if a given feature is supported in the config. One method to
check based on boolean values (as is being done today for virt
features), and nother to check based on a range (from, to) which I
would
like to use for gluster features.
2) Renamed FeatureSupported to VirtFeatureSupported, and made it use
the
first utility method from CompatibilityUtils
3) Introduced GlusterFeatureSupported for gluster features, which
uses
the second utility method from CompatibilityUtils
Key advantage here is that
- we don't have to touch any virt specifc source for adding
compatibility checks for gluster features
- virt features continue to use the existing boolean config check
Any comments / suggestions / reviews will be highly appreciated :)
</pre>
</blockquote>
<pre>I think splitting to two classes is OK, but the underlying
mechanism IMO should be as Omer suggested:
Use the default value from the java config file, and if in the DB there is a version
specific value then use it for that version only.</pre>
</blockquote>
<br>
Review comments here are on the contrary:<br>
<a moz-do-not-send="true"
href="http://gerrit.ovirt.org/#/c/12970/5/backend/manager/dbscripts/...
target="_blank"
data-mce-href="http://gerrit.ovirt.org/#/c/12970/5/backend/manager/d...
data-mce-bogus="1">
</blockquote>
<div>The comment in the review simply states that the mechanism
is probably broken, not that that's the way it has to be done.<br>
</div>
</div>
</blockquote>
<br>
The comment explicitly asks me to add entries for every version.
What you have looked at is my response to this comment, which
suggests that the current mechanism is not great. In fact, what I
had done in patch-set 5 is exactly what you are suggesting : true as
default value and explicit entries in config for the "false" values.
But it was not accepted. <br>
<br>
<blockquote
cite="mid:1842623267.574910.1364894251176.JavaMail.root@redhat.com"
type="cite">
<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;"
data-mce-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>
<blockquote
cite="mid:36623975.7044697.1364386718971.JavaMail.root@redhat.com">
<pre>I don't think "From, To, etc" is a good design,
it's not a standard way and is very restrictive.</pre>
</blockquote>
<br>
Can you please explain in what way is it restrictive?<br>
<br>
Also, what is the "etc" you are referring to?</blockquote>
<div>What if for certain version it is not supported, you add
"except"? Or do you specify 2 ranges?<br>
</div>
<div><br>
</div>
<div>Starting to add from/to creates a limited design of one
range, which would be difficult to tune if necessary.</div>
</div>
</blockquote>
<br>
Really? Does someone really think that there will be a feature that
will be supported in multiple different ranges of versions? I see
zero possibility for this. I would love to see some +1s on this
concept before I can accept this argument.<br>
<br>
<blockquote
cite="mid:1842623267.574910.1364894251176.JavaMail.root@redhat.com"
type="cite">
<div style="font-family: times new roman, new york, times, serif;
font-size: 12pt; color: #000000">
<div>I think the design generally for config values is very
simple and suits us well - use the default value, unless a
specific version is configured differently.<br>
</div>
</div>
</blockquote>
<br>
I think the current design is wrong. A feature gets supported "from"
a particular version, and that's all that is required in most of the
cases. Expecting developers to add version-by-version mapping for
features is bad. The "to" part in my patch is just to handle rare
cases, if at all they come up. I'm willing to even remove that if
such a case doesn't exist today.<br>
<br>
Also, even though I have followed it for the sake of consistency, I
don't think these values need to be stored in the config (db) at
all. Only explanation I've got for it is that it was probably
'convenient' for developers to use the config mechanism. I'm for
having this check purely in code in a central place, and not the
config (db).<br>
<br>
<blockquote
cite="mid:1842623267.574910.1364894251176.JavaMail.root@redhat.com"
type="cite">
<div style="font-family: times new roman, new york, times, serif;
font-size: 12pt; color: #000000">
<div>This way you can specify the feature is supported, and
disable it for specific versions.<br>
</div>
</div>
</blockquote>
<br>
So one has to look at both code (FeatureSupported) as well as db
(config) to get an idea of what versions the feature is supported
in. Not great. <br>
<br>
<blockquote
cite="mid:1842623267.574910.1364894251176.JavaMail.root@redhat.com"
type="cite">
<div style="font-family: times new roman, new york, times, serif;
font-size: 12pt; color: #000000">
<div>I think this direction gives us the flexibility that we
would like to have.<br>
</div>
<div><br>
</div>
<div>Currently it doesn't work that way, but I think it's not
impossible to change, and more worthwhile than introducing a
new mechanism.<br>
</div>
</div>
</blockquote>
<br>
I disagree, and would like to use the "supported from" mechanism at
least for gluster features.<br>
<br>
<blockquote
cite="mid:1842623267.574910.1364894251176.JavaMail.root@redhat.com"
type="cite">
<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;"
data-mce-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>
<blockquote
cite="mid:36623975.7044697.1364386718971.JavaMail.root@redhat.com">
<blockquote>
<blockquote>
<blockquote>
<blockquote>
<blockquote>
<blockquote>
<pre>Thoughts?
Regards,
Shireesh
</pre>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
<br>
</blockquote>
<div><br>
</div>
</div>
</blockquote>
<br>
</body>
</html>
--------------060100050808070407030307--
1:46 a.m.
This is a multi-part message in MIME format.
--------------090808090708060603070606
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
The idea should be to make sure that maintaining the feature supported
matrix is not a nightmare. If we need to go and replicate entries in
0000_config.sql file for each new version, then this is an issue. And I
think we are all in agreement that this is not the way to go.
Either we go with
[1]. default value in ConfigValues, and only changed value in db script
like in patch set 5 (http://gerrit.ovirt.org/#/c/12970/5)
If this mechanism is broken, do you know where/what is broken?
or [2] . with the new approach where a Feature supported changes to
true/false **from** a particular version.
(I think, for gluster features, the Feature From works for us as we do
not see it changing from version to version once supported. )
But if there's a way to fix 1, let's do that to get this moving.
Mike, could you elaborate on what needs to be fixed in [1] ?
On 04/02/2013 04:30 PM, Shireesh Anjal wrote:
On 04/02/2013 02:47 PM, Mike Kolesnik wrote:
> ------------------------------------------------------------------------
>
> On 03/27/2013 05:48 PM, Mike Kolesnik wrote:
>
> ----- Original Message -----
>
> On 03/20/2013 08:20 PM, Yair Zaslavsky wrote:
>
> ----- Original Message -----
>
> From: "Shireesh Anjal"<sanjal(a)redhat.com>
> To: "Mike Kolesnik"<mkolesni(a)redhat.com>
> Cc:engine-devel@ovirt.org
> Sent: Wednesday, March 20, 2013 4:47:08 PM
> Subject: Re: [Engine-devel] FeatureSupported and compatibility
> versions
>
> On 03/18/2013 01:11 PM, Shireesh Anjal wrote:
>
> On 03/18/2013 12:59 PM, Mike Kolesnik wrote:
>
> ----- Original Message -----
>
> 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:
>
> You can "merge" the configs into a single
config when older
> versions
> go out of the supported versions for the system.
>
> i.e. in 4.0 you can have upgrade script that merges all
> GlusterFeatureSupported to one entry instead of several.
>
> Why are we even storing this information in config? Is this
> something
> that can be "configured" at customer site?
>
> As previously explained (but off list :) ) , Config gives you the
> ability to have a cachable "map" of entry (i.e -
"feature name")
> per version and value.
> I guess it was convinient for the developers to use that.
> I also mentioned that customers/oVirt users should config the
> entries of vdc_options using engine-config tool only.
> Not all entries are exposed via engine-config.properties (and no,
> not just "is feature supported" entries are hidden).
>
>
>
>
> 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'm not sure why we would want to complicate this
simple
> mechanism?
>
> Is there much to gain?
>
> I think option 1 suggested above is simpler - to implement
as
> well
> as
> to understand.
>
> Let me give you an example of why I don't like current
mechanism.
> I
> introduce a version check for a feature that was introduced
in
> 3.1.
> I'm being asked now to add three entries in config
>
> 3.0 - false
> 3.1 - true
> 3.2 - true
>
> It will also mean that when 3.3 goes out, someone has to
make
> sure
> that another entry is added for 3.3-true. I think it is not
> logical
> as
> well as scalable if you have more versions. And it sounds
far
> more
> complex (to maintain) than just having
>
> <Feature>SupportedFrom = 3.1
>
> So I would like to know if there are any objections to my
> proposal.
> I
> intend to use this for at least the gluster related
features.
>
> I've sent a patch (http://gerrit.ovirt.org/12970) with following
> changes:
>
> 1) Introduced CompatibilityUtils that provides utility methods for
> checking if a given feature is supported in the config. One method to
> check based on boolean values (as is being done today for virt
> features), and nother to check based on a range (from, to) which I
> would
> like to use for gluster features.
> 2) Renamed FeatureSupported to VirtFeatureSupported, and made it use
> the
> first utility method from CompatibilityUtils
> 3) Introduced GlusterFeatureSupported for gluster features, which
> uses
> the second utility method from CompatibilityUtils
>
> Key advantage here is that
> - we don't have to touch any virt specifc source for adding
> compatibility checks for gluster features
> - virt features continue to use the existing boolean config check
>
> Any comments / suggestions / reviews will be highly appreciated :)
>
> I think splitting to two classes is OK, but the underlying mechanism IMO
should be as Omer suggested:
> Use the default value from the java config file, and if in the DB there is a
version specific value then use it for that version only.
>
>
> Review comments here are on the contrary:
>
http://gerrit.ovirt.org/#/c/12970/5/backend/manager/dbscripts/upgrade/pre...
>
> The comment in the review simply states that the mechanism is
> probably broken, not that that's the way it has to be done.
The comment explicitly asks me to add entries for every version. What
you have looked at is my response to this comment, which suggests that
the current mechanism is not great. In fact, what I had done in
patch-set 5 is exactly what you are suggesting : true as default value
and explicit entries in config for the "false" values. But it was not
accepted.
>
> I don't think "From, To, etc" is a good design, it's not a
standard way and is very restrictive.
>
>
> Can you please explain in what way is it restrictive?
>
> Also, what is the "etc" you are referring to?
>
> What if for certain version it is not supported, you add "except"? Or
> do you specify 2 ranges?
>
> Starting to add from/to creates a limited design of one range, which
> would be difficult to tune if necessary.
Really? Does someone really think that there will be a feature that
will be supported in multiple different ranges of versions? I see zero
possibility for this. I would love to see some +1s on this concept
before I can accept this argument.
> I think the design generally for config values is very simple and
> suits us well - use the default value, unless a specific version is
> configured differently.
I think the current design is wrong. A feature gets supported "from" a
particular version, and that's all that is required in most of the
cases. Expecting developers to add version-by-version mapping for
features is bad. The "to" part in my patch is just to handle rare
cases, if at all they come up. I'm willing to even remove that if such
a case doesn't exist today.
Also, even though I have followed it for the sake of consistency, I
don't think these values need to be stored in the config (db) at all.
Only explanation I've got for it is that it was probably 'convenient'
for developers to use the config mechanism. I'm for having this check
purely in code in a central place, and not the config (db).
> This way you can specify the feature is supported, and disable it for
> specific versions.
So one has to look at both code (FeatureSupported) as well as db
(config) to get an idea of what versions the feature is supported in.
Not great.
> I think this direction gives us the flexibility that we would like to
> have.
>
> Currently it doesn't work that way, but I think it's not impossible
> to change, and more worthwhile than introducing a new mechanism.
I disagree, and would like to use the "supported from" mechanism at
least for gluster features.
>
> Thoughts?
>
> Regards,
> Shireesh
>
>
>
_______________________________________________
Engine-devel mailing list
Engine-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
--------------090808090708060603070606
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">
The idea should be to make sure that maintaining the feature
supported matrix is not a nightmare. If we need to go and replicate
entries in 0000_config.sql file for each new version, then this is
an issue. And I think we are all in agreement that this is not the
way to go.<br>
<br>
Either we go with <br>
[1]. default value in ConfigValues, and only changed value in db
script like in patch set 5 (<a class="moz-txt-link-freetext"
href="http://gerrit.ovirt.org/#/c/12970/5">http://gerrit.ovi...
<br>
If this mechanism is broken, do you know where/what is broken?<br>
<br>
or [2] . with the new approach where a Feature supported changes to
true/false <b>*from*</b> a particular version.<br>
(I think, for gluster features, the Feature From works for us as we
do not see it changing from version to version once supported. )<br>
<br>
But if there's a way to fix 1, let's do that to get this moving.<br>
Mike, could you elaborate on what needs to be fixed in [1] ?<br>
<br>
<br>
<br>
<div class="moz-cite-prefix">On 04/02/2013 04:30 PM, Shireesh Anjal
wrote:<br>
</div>
<blockquote cite="mid:515ABA63.4070106@redhat.com"
type="cite">
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
<div class="moz-cite-prefix">On 04/02/2013 02:47 PM, Mike Kolesnik
wrote:<br>
</div>
<blockquote
cite="mid:1842623267.574910.1364894251176.JavaMail.root@redhat.com"
type="cite">
<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;"
data-mce-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 class="moz-cite-prefix">On 03/27/2013 05:48 PM, Mike
Kolesnik wrote:<br>
</div>
<blockquote
cite="mid:36623975.7044697.1364386718971.JavaMail.root@redhat.com">
<pre>----- Original Message -----
</pre>
<blockquote>
<pre>On 03/20/2013 08:20 PM, Yair Zaslavsky wrote:
</pre>
<blockquote>
<pre>----- Original Message -----
</pre>
<blockquote>
<pre>From: "Shireesh Anjal" <a
moz-do-not-send="true" class="moz-txt-link-rfc2396E"
href="mailto:sanjal@redhat.com" target="_blank"
data-mce-href="mailto:sanjal@redhat.com"><sanjal@redhat.com></a>
To: "Mike Kolesnik" <a moz-do-not-send="true"
class="moz-txt-link-rfc2396E" href="mailto:mkolesni@redhat.com"
target="_blank"
data-mce-href="mailto:mkolesni@redhat.com"><mkolesni@redhat.com></a>
Cc: <a moz-do-not-send="true" class="moz-txt-link-abbreviated"
href="mailto:engine-devel@ovirt.org" target="_blank"
data-mce-href="mailto:engine-devel@ovirt.org">engine-devel@ovirt.org</a>
Sent: Wednesday, March 20, 2013 4:47:08 PM
Subject: Re: [Engine-devel] FeatureSupported and compatibility
versions
On 03/18/2013 01:11 PM, Shireesh Anjal wrote:
</pre>
<blockquote>
<pre>On 03/18/2013 12:59 PM, Mike Kolesnik wrote:
</pre>
<blockquote>
<pre>----- Original Message -----
</pre>
<blockquote>
<pre>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:
</pre>
</blockquote>
<pre>You can "merge" the configs into a single
config when older
versions
go out of the supported versions for the system.
i.e. in 4.0 you can have upgrade script that merges all
GlusterFeatureSupported to one entry instead of several.
</pre>
</blockquote>
</blockquote>
<pre>Why are we even storing this information in config? Is
this
something
that can be "configured" at customer site?
</pre>
</blockquote>
<pre>As previously explained (but off list :) ) , Config gives you
the
ability to have a cachable "map" of entry (i.e - "feature name")
per version and value.
I guess it was convinient for the developers to use that.
I also mentioned that customers/oVirt users should config the
entries of vdc_options using engine-config tool only.
Not all entries are exposed via engine-config.properties (and no,
not just "is feature supported" entries are hidden).
</pre>
<blockquote>
<blockquote>
<blockquote>
<blockquote>
<pre>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)
</pre>
</blockquote>
<pre>I'm not sure why we would want to complicate this
simple
mechanism?
Is there much to gain?
</pre>
</blockquote>
<pre>I think option 1 suggested above is simpler - to
implement as
well
as
to understand.
Let me give you an example of why I don't like current mechanism.
I
introduce a version check for a feature that was introduced in
3.1.
I'm being asked now to add three entries in config
3.0 - false
3.1 - true
3.2 - true
It will also mean that when 3.3 goes out, someone has to make
sure
that another entry is added for 3.3-true. I think it is not
logical
as
well as scalable if you have more versions. And it sounds far
more
complex (to maintain) than just having
<Feature>SupportedFrom = 3.1
So I would like to know if there are any objections to my
proposal.
I
intend to use this for at least the gluster related features.
</pre>
</blockquote>
</blockquote>
</blockquote>
<pre>I've sent a patch (<a moz-do-not-send="true"
class="moz-txt-link-freetext" href="http://gerrit.ovirt.org/12970"
target="_blank"
data-mce-href="http://gerrit.ovirt.org/12970">http://gerrit....>)
with following
changes:
1) Introduced CompatibilityUtils that provides utility methods for
checking if a given feature is supported in the config. One method to
check based on boolean values (as is being done today for virt
features), and nother to check based on a range (from, to) which I
would
like to use for gluster features.
2) Renamed FeatureSupported to VirtFeatureSupported, and made it use
the
first utility method from CompatibilityUtils
3) Introduced GlusterFeatureSupported for gluster features, which
uses
the second utility method from CompatibilityUtils
Key advantage here is that
- we don't have to touch any virt specifc source for adding
compatibility checks for gluster features
- virt features continue to use the existing boolean config check
Any comments / suggestions / reviews will be highly appreciated :)
</pre>
</blockquote>
<pre>I think splitting to two classes is OK, but the underlying
mechanism IMO should be as Omer suggested:
Use the default value from the java config file, and if in the DB there is a version
specific value then use it for that version only.</pre>
</blockquote>
<br>
Review comments here are on the contrary:<br>
<a moz-do-not-send="true"
href="http://gerrit.ovirt.org/#/c/12970/5/backend/manager/dbscripts/...
target="_blank"
data-mce-href="http://gerrit.ovirt.org/#/c/12970/5/backend/manager/d...
data-mce-bogus="1">
</blockquote>
<div>The comment in the review simply states that the
mechanism is probably broken, not that that's the way it has
to be done.<br>
</div>
</div>
</blockquote>
<br>
The comment explicitly asks me to add entries for every version.
What you have looked at is my response to this comment, which
suggests that the current mechanism is not great. In fact, what I
had done in patch-set 5 is exactly what you are suggesting : true
as default value and explicit entries in config for the "false"
values. But it was not accepted. <br>
<br>
<blockquote
cite="mid:1842623267.574910.1364894251176.JavaMail.root@redhat.com"
type="cite">
<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;"
data-mce-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>
<blockquote
cite="mid:36623975.7044697.1364386718971.JavaMail.root@redhat.com">
<pre>I don't think "From, To, etc" is a good design,
it's not a standard way and is very restrictive.</pre>
</blockquote>
<br>
Can you please explain in what way is it restrictive?<br>
<br>
Also, what is the "etc" you are referring to?</blockquote>
<div>What if for certain version it is not supported, you add
"except"? Or do you specify 2 ranges?<br>
</div>
<div><br>
</div>
<div>Starting to add from/to creates a limited design of one
range, which would be difficult to tune if necessary.</div>
</div>
</blockquote>
<br>
Really? Does someone really think that there will be a feature
that will be supported in multiple different ranges of versions? I
see zero possibility for this. I would love to see some +1s on
this concept before I can accept this argument.<br>
<br>
<blockquote
cite="mid:1842623267.574910.1364894251176.JavaMail.root@redhat.com"
type="cite">
<div style="font-family: times new roman, new york, times,
serif; font-size: 12pt; color: #000000">
<div>I think the design generally for config values is very
simple and suits us well - use the default value, unless a
specific version is configured differently.<br>
</div>
</div>
</blockquote>
<br>
I think the current design is wrong. A feature gets supported
"from" a particular version, and that's all that is required in
most of the cases. Expecting developers to add version-by-version
mapping for features is bad. The "to" part in my patch is just to
handle rare cases, if at all they come up. I'm willing to even
remove that if such a case doesn't exist today.<br>
<br>
Also, even though I have followed it for the sake of consistency,
I don't think these values need to be stored in the config (db) at
all. Only explanation I've got for it is that it was probably
'convenient' for developers to use the config mechanism. I'm for
having this check purely in code in a central place, and not the
config (db).<br>
<br>
<blockquote
cite="mid:1842623267.574910.1364894251176.JavaMail.root@redhat.com"
type="cite">
<div style="font-family: times new roman, new york, times,
serif; font-size: 12pt; color: #000000">
<div>This way you can specify the feature is supported, and
disable it for specific versions.<br>
</div>
</div>
</blockquote>
<br>
So one has to look at both code (FeatureSupported) as well as db
(config) to get an idea of what versions the feature is supported
in. Not great. <br>
<br>
<blockquote
cite="mid:1842623267.574910.1364894251176.JavaMail.root@redhat.com"
type="cite">
<div style="font-family: times new roman, new york, times,
serif; font-size: 12pt; color: #000000">
<div>I think this direction gives us the flexibility that we
would like to have.<br>
</div>
<div><br>
</div>
<div>Currently it doesn't work that way, but I think it's not
impossible to change, and more worthwhile than introducing a
new mechanism.<br>
</div>
</div>
</blockquote>
<br>
I disagree, and would like to use the "supported from" mechanism
at least for gluster features.<br>
<br>
<blockquote
cite="mid:1842623267.574910.1364894251176.JavaMail.root@redhat.com"
type="cite">
<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;"
data-mce-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>
<blockquote
cite="mid:36623975.7044697.1364386718971.JavaMail.root@redhat.com">
<blockquote>
<blockquote>
<blockquote>
<blockquote>
<blockquote>
<blockquote>
<pre>Thoughts?
Regards,
Shireesh
</pre>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
<br>
</blockquote>
<div><br>
</div>
</div>
</blockquote>
<br>
<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>
--------------090808090708060603070606--
4:14 p.m.
On 03/20/2013 04:47 PM, Shireesh Anjal wrote:
Why are we even storing this information in config? Is this
something
that can be "configured" at customer site?
some features can be disabled, and only enabled by someone aware of the
implications of enabling them. that's apart of the versions they are
supported at i guess
3 a.m.
----- Original Message -----
From: "Shireesh Anjal" <sanjal(a)redhat.com>
To: engine-devel(a)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.
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.
Thoughts?
Regards,
Shireesh
_______________________________________________
Engine-devel mailing list
Engine-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
5:03 a.m.
On 03/18/2013 01:30 PM, Omer Frenkel wrote:
----- Original Message -----
> From: "Shireesh Anjal" <sanjal(a)redhat.com>
> To: engine-devel(a)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(a)ovirt.org
> http://lists.ovirt.org/mailman/listinfo/engine-devel
>
5:23 a.m.
----- Original Message -----
From: "Shireesh Anjal" <sanjal(a)redhat.com>
To: engine-devel(a)ovirt.org
Cc: "Omer Frenkel" <ofrenkel(a)redhat.com>
Sent: Thursday, May 2, 2013 1:03:23 PM
Subject: Re: [Engine-devel] FeatureSupported and compatibility versions
On 03/18/2013 01:30 PM, Omer Frenkel wrote:
>
> ----- Original Message -----
>> From: "Shireesh Anjal" <sanjal(a)redhat.com>
>> To: engine-devel(a)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.
Thanks!
>
>> Thoughts?
>>
>> Regards,
>> Shireesh
>> _______________________________________________
>> Engine-devel mailing list
>> Engine-devel(a)ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/engine-devel
>>
4252
days inactive
4297
days old
16 comments
7 participants
participants (7)
-
Itamar Heim -
Kanagaraj Mayilsamy -
Mike Kolesnik -
Omer Frenkel -
Sahina Bose -
Shireesh Anjal -
Yair Zaslavsky