Vered Volansky has posted comments on this change.
Change subject: core:WIP: introducing OvfAutoUpdate
......................................................................
Patch Set 10: (17 inline comments)
....................................................
File backend/manager/dbscripts/create_views.sql
Line 373: vds_groups ON vm_templates.vds_group_id = vds_groups.vds_group_id
Line 374: left outer JOIN
Line 375: storage_pool ON storage_pool.id = vds_groups.storage_pool_id
Line 376: left outer JOIN
Line 377: quota ON vm_templates.quota_id = quota.id
Maybe it's just gerrit, but looks like a trailing space was added here.
Line 378: WHERE entity_type = 'TEMPLATE';
Line 379:
Line 380:
Line 381:
....................................................
File backend/manager/dbscripts/upgrade/03_01_1530_add_vm_generation_columns.sql
Line 1: select fn_db_add_column('vm_static', 'db_generation', 'BIGINT default
1');
Line 2:
Line 3: -- not added as foreign key so that when vm is removed, it record in
vm_ovf_generations record will stay
s/it/its
Line 4: CREATE TABLE vm_ovf_generations
Line 5: (
Line 6: vm_guid UUID PRIMARY KEY,
Line 7: storage_pool_id UUID references storage_pool(id) ON DELETE CASCADE,
Line 9: );
Line 10:
Line 11: INSERT into vm_ovf_generations (select vm.vm_guid, sp.id from
vm_static vm ,storage_pool sp, vds_groups vg where vg.storage_pool_id = sp.id
AND vm.vds_group_id = vg.vds_group_id);
Line 12:
Line 13: -- ovf_generation of 1 should be set only the pre existing vms, so
after we added
s/only the/only for the.
Or just totaly rephrase (preferable)-
Only pre-existing vms should have ovf_generation set to 1, so ...
Line 14: -- the pre existing vms, the default should be 0.
Line 15: ALTER TABLE vm_ovf_generations ALTER COLUMN ovf_generation set default
0;
....................................................
File backend/manager/dbscripts/vms_sp.sql
Line 18: FETCH curs_vmids INTO id;
Line 19: FETCH curs_newovfgen INTO new_ovf_gen;
Line 20: IF NOT FOUND THEN
Line 21: EXIT;
Line 22: END IF;
trailin spaces.
Line 23: UPDATE vm_ovf_generations
Line 24: SET ovf_generation = new_ovf_gen WHERE vm_guid = id;
Line 25: END LOOP;
Line 26: CLOSE curs_vmids;
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/OvfDataUpdater.java
Line 107: }
Line 108: }
Line 109:
Line 110: /**
Line 111: * update ovfs for updated/added vms since last for the given
storage pool
Unclear comment, please rephrase
Line 112: * @param poolId
Line 113: */
Line 114: protected void updateOvfForVmsOfStoragePool(Guid poolId) {
Line 115: // get vm ids that needs to be updated.
Line 111: * update ovfs for updated/added vms since last for the given
storage pool
Line 112: * @param poolId
Line 113: */
Line 114: protected void updateOvfForVmsOfStoragePool(Guid poolId) {
Line 115: // get vm ids that needs to be updated.
s/needs/need
Line 116: List<Guid> vmsIdsForUpdate =
getVmAndTemplatesGenerationsDao().getVmsIdsForOvfUpdate(poolId);
Line 117: int i = 0;
Line 118: while (i < vmsIdsForUpdate.size()) {
Line 119: int delta = vmsIdsForUpdate.size() - i;
Line 147: // update vm/templates ovf generation to the db generation
that was updated in the storage.
Line 148:
getVmAndTemplatesGenerationsDao().updateOvfGenerations(proccessedIdsInfo,
proccessedOvfGenerationsInfo);
Line 149: proccessedIdsInfo.clear();
Line 150: proccessedOvfGenerationsInfo.clear();
Line 151: vmsAndTemplateMetadata.clear();
I'd extract the last three operations into a 'clear' method
Line 152: }
Line 153:
Line 154: /**
Line 155: * returns a list of templates that are valid for update from the
given templates list.
Line 152: }
Line 153:
Line 154: /**
Line 155: * returns a list of templates that are valid for update from the
given templates list.
Line 156: * valid template is a template which is not locked and none of
it's disks is locked.
s/which/that
s/disks is/disks are
Line 157: * @param templates
Line 158: * @return
Line 159: */
Line 160: protected void populateTemplatesMetadataForOvfUpdate(List<Guid>
idsToProcess) {
Line 208: for (VM vm : vms) {
Line 209: if (VMStatus.ImageLocked != vm.getstatus()) {
Line 210: VmHandler.updateDisksFromDb(vm);
Line 211: boolean verifyDisksNotLocked =
verifyDisksNotLocked(vm.getDiskList());
Line 212: if (verifyDisksNotLocked) {
Why the extraction into a variable, it's short as it is and is not used again
Line 213: loadVmData(vm);
Line 214: Long currentDbGeneration =
getVmStaticDao().getDbGeneration(vm.getId());
Line 215: if (vm.getStaticData().getDb_generation() ==
currentDbGeneration.longValue()) {
Line 216: buildMetadataDictionaryForVm(vm,
vmsAndTemplateMetadata);
Line 249: }
Line 250:
Line 251: protected void loadTemplateData(VmTemplate template) {
Line 252: if (template.getInterfaces() == null ||
template.getInterfaces().isEmpty()) {
Line 253:
template.setInterfaces(DbFacade.getInstance().getVmNetworkInterfaceDao()
Extract DbFacade.getInstance().getVmNetworkInterfaceDao() into a method and use
bellow as well.
Line 254: .getAllForTemplate(template.getId()));
Line 255: }
Line 256: }
Line 257:
Line 265: }
Line 266: }
Line 267:
Line 268: protected void buildMetadataDictionaryForVm(VM vm , Map<Guid,
KeyValuePairCompat<String, List<Guid>>> metaDictionary){
Line 269: ArrayList<DiskImage> AllVmImages = new ArrayList<DiskImage>();
should use List on the left hand side and java naming convention.
Line 270: for (Disk disk : vm.getDiskMap().values()) {
Line 271: if (disk.isAllowSnapshot()) {
Line 272: DiskImage diskImage = (DiskImage) disk;
Line 273:
AllVmImages.addAll(ImagesHandler.getAllImageSnapshots(diskImage.getImageId(),
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmDAO.java
Line 159: */
Line 160: public List<VM> getAllVmsRelatedToQuotaId(Guid quotaId);
Line 161:
Line 162: /**
Line 163: * Get all vms which were updated in db since last ovf update in
storage by ids.
s/which/that. I know it seems petty, but it's grammatically incorrect this way.
Thanks.
Line 164: *
Line 165: * @param quotaId
Line 166: * @param vmCount
Line 167: * @return
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmStaticDAO.java
Line 76: */
Line 77: public void incrementDbGenerationForAllInStoragePool(Guid
storagePoolId);
Line 78:
Line 79: /**
Line 80: * increment the generation for the vm/template with the given
guid by 1
s/increment the generation for the vm/template with the given guid by
1/increment by 1 the generation of the vm/template with the given guid
Line 81: *
Line 82: * @param id - vm/template id
Line 83: * @return
Line 84: */
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmTemplateDAO.java
Line 77: */
Line 78: List<VmTemplate> getAllTemplatesRelatedToQuotaId(Guid quotaId);
Line 79:
Line 80: /**
Line 81: * Get all templates which were updated in db since last ovf
update in storage by ids.
s/which/that
Line 82: *
Line 83: * @param quotaId
Line 84: * @param teamplatesCount
Line 85: * @return
....................................................
Commit Message
Line 20: updates will be done in specifed time intervals which will reduce
Line 21: XML-RPC calls and will enable the removal of this syncronous vdsm call
Line 22: from all over the code.
Line 23:
Line 24: *vdsm operation for removal of multiple vms/templates ovfs at once
need to be
s/need/needs
Line 25: implemented
Line 26:
Line 27: * loadVm/VmTemplateData methods need to be revisited, if clauses in it
Line 28: might be irrelevant
Line 23:
Line 24: *vdsm operation for removal of multiple vms/templates ovfs at once
need to be
Line 25: implemented
Line 26:
Line 27: * loadVm/VmTemplateData methods need to be revisited, if clauses in it
s/if//
Line 28: might be irrelevant
Line 29:
Line 30: *UpdateVMVdsCommand error should be catched.
Line 31:
Line 26:
Line 27: * loadVm/VmTemplateData methods need to be revisited, if clauses in it
Line 28: might be irrelevant
Line 29:
Line 30: *UpdateVMVdsCommand error should be catched.
s/catched/caught
Line 31:
Line 32: Change-Id: I9b5132300fb1f1fd94f771cab15efe5246dbeca8
--
To view, visit http://gerrit.ovirt.org/9328
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9b5132300fb1f1fd94f771cab15efe5246dbeca8
Gerrit-PatchSet: 10
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot <lara...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Arik Hadas <aha...@redhat.com>
Gerrit-Reviewer: Ayal Baron <aba...@redhat.com>
Gerrit-Reviewer: Liron Aravot <lara...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Michael Kublin <mkub...@redhat.com>
Gerrit-Reviewer: Tal Nisan <tni...@redhat.com>
Gerrit-Reviewer: Vered Volansky <vvola...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: liron aravot <liron.ara...@gmail.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
