Re: [Engine-devel] Introducing virt / gluster flags at cluster level

----- Original Message ----- > From: "Moti Asayag" <masayag(a)redhat.com&gt; > To: engine-devel(a)ovirt.org > Sent: Sunday, March 4, 2012 12:20:43 PM > Subject: Re: [Engine-devel] Introducing virt / gluster flags at > cluster level > > On 03/01/2012 12:54 PM, Shireesh Anjal wrote: > > Hi, > > > > In order to identify whether a cluster exposes Gluster / > > Virtualization > > capabilities, we plan to introduce two boolean columns - > > virt_service > > and gluster_service in the vds_groups table. As per immediate > > plans, it > > is intended to support only one service per cluster, meaning only > > one of > > these two values can be true. > > Couldn't there be additional future services in the future ? In > that > case perhaps worth considering enum for services, to be stored in a > single service column, its values are: virt, gluster,...) instead > of > extending the vds_group table consistently when introducing new > services > (under the assumption no mix of services is allowed). +1 for an Enum insteand of boolean columns. we have too much of that already and eventually we see long records and routine refactoring to our DAL. Also to make mixed configuration we can embrace bit fields which interacts very nice with Enums e.g a 5 value of is a cluster with VIRT(1) and FUTURE(4) capabilities

> > > > > We plan to make following changes as part of the patch: > > > > 1) Introduce an upgrade script for adding following columns to > > the > > table > > "vds_groups" > > - virt_service - boolean - NOT_NULL - default value true > > - gluster_service - boolean - NOT_NULL - default value false > > > > => What is the naming convention for naming the upgrade script? > > Is > > it > > <major-version>_<minor-version>_<running-sequence-with-step-10>_script_description.sql > > ? By that logic, the name of our script will be something like > > 03_01_0560_add_service_columns_to_vds_groups.sql > > Correct. Make sure to verify script sequence uniqueness (under > ovirt-engine/backend/manager/dbscripts/upgrade) before pushing, > since > there is no enforcement for that. > > > > > 2) Modify vds_groups_sp.sql to introduce these arguments / > > variables in > > save and update methods > > > > 3) Modify the DAO interface and implementation to introduce these > > arguments / variables in the save/update methods and > > VdsGroupRawMapper. > > > > 4) Modify the DAO JUnit test class to take care of these new > > fields, if > > required. > > As part of DAO unit-test you should add the new columns and > possible > data to > ovirt-engine/backend/manager/modules/dal/src/test/resources/fixtures.xml > > > > > 5) Modify class VDSGroup to introduce the two new boolean > > variables > > virtService and glusterService and modify the methods equals and > > hashCode to use these. > > > > => One question here. Most of the attributes of class VDSGroup > > have > > annotations like @XmlElement and @Column. I think these are > > related > > to > > jaxb and JPA. Are these annotations really required? If yes, how > > are > > they useful? > > > > @XmlElement annotations could be deleted from that file as a prior > patch > to the patch-set. > > JPA annotations could be ignored as not fully supported nor > maintained. > > > 6) REST API : Default populate these variables (virtService = > > true, > > glusterService = false) before invoking the BLL command > > (AddVdsGroupCommand / UpdateVdsGroupCommand). This makes sure > > that > > the > > existing API won't break. No change in api.xsd for now. > > > > 7) webadmin create cluster code: Default populate the new > > variables, > > same as above. No change in UI screen for now. > > > > => Alternatively, the variable declaration on VDSGroup itself can > > initialize these variables with default values, so that changes > > to > > UI / > > REST API code (points 6 and 7) may not be required immediately. > > What do > > you suggest? > > > > Inputs / comments / suggestions welcome. > > > > _______________________________________________ > 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