Michael Kublin has posted comments on this change.

Change subject: core:WIP: introducing OvfAutoUpdate
......................................................................


Patch Set 3: (5 inline comments)

Not all question is answered

....................................................
File backend/manager/dbscripts/create_views.sql
Line 325: 
Line 326: 
Line 327: CREATE OR REPLACE VIEW vm_templates_view
Line 328: AS
Line 329: 
View is not refreshes when some table of view is changed?
Line 330: SELECT
Line 331: vm_templates.vm_guid as vmt_guid,
Line 332:        vm_templates.vm_name as name,
Line 333:        vm_templates.mem_size_mb as mem_size_mb,


....................................................
File backend/manager/dbscripts/upgrade/03_01_1490_add_vm_generation_columns.sql
Line 1: select fn_db_add_column('vm_static', 'ovf_generation', 'BIGINT');
Line 2: select fn_db_add_column('vm_static', 'db_generation', 'BIGINT default 
1');
It is not,  vm_statistics or vm_dynamic it is related to update of vms
Line 3: UPDATE vm_static set ovf_generation = 1;


....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/OvfDataUpdater.java
Line 65:                         }
Line 66:                     }
Line 67:                     List<VmTemplate> templatesForUpdate = 
getTemplatesForUpdate(DbFacade.getInstance()
Line 68:                             .getVmTemplateDao()
Line 69:                             
.getAllVmTemplatesForOvfUpdateForStoragePool(pool.getId()));
So before these your code will crash a system? 
Also you don't need to keep vms and templates together in memory and these is 
not related to any chunks.
Line 70:                     if (!templatesForUpdate.isEmpty()) {
Line 71:                         
VmTemplateCommand.UpdateTemplateInSpm(pool.getId(), templatesForUpdate);
Line 72:                         for (VmTemplate template : templatesForUpdate) 
{
Line 73:                             DbFacade.getInstance()


Line 89:      * returns a list of vms that are valid for update from the given 
vm list.
Line 90:      * valid vm is a vm which is not locked and none of it's disks is 
locked.
Line 91:      * @param vms
Line 92:      * @return
Line 93:      */
"as soon as chunks split will be added this will be avoided." - and who should 
do these?
Line 94:     private List<VM> getVmsForUpdate(List<VM> vms) {
Line 95:         List<VM> toReturn = new ArrayList<VM>();
Line 96:         for (VM vm : vms) {
Line 97:             if (VMStatus.ImageLocked != vm.getstatus()) {


Line 131:      * @return
Line 132:      */
Line 133:     private boolean verifyDisksNotLocked (List<DiskImage> disks) {
Line 134:         for (DiskImage disk : disks) {
Line 135:             if (disk.getimageStatus() == ImageStatus.LOCKED) {
These check is useless..
Line 136:                 return false;
Line 137:             }
Line 138:         }
Line 139:         return true;


--
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: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot <lara...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@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

Reply via email to