Michael Kublin has posted comments on this change.

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


Patch Set 3: (7 inline comments)

Discussed couple of issues , should be solved at next patch

....................................................
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');
Because of templates, possible better to have different table
Line 3: UPDATE vm_static set ovf_generation = 1;


....................................................
File backend/manager/dbscripts/upgrade/pre_upgrade/0000_config.sql
Line 548: select 
fn_db_add_config_value('VmPoolMonitorMaxAttempts','3','general');
Line 549: select fn_db_add_config_value('VmPriorityMaxValue','100','general');
Line 550: --Handling Keyboard Layout configuration for VNC
Line 551: select fn_db_add_config_value('VncKeyboardLayout','en-us','general');
Line 552: select fn_db_add_config_value('WaitForVdsInitInSec','60','general');
I prefer default value at least 10 minutes
Line 553: select 
fn_db_add_config_value('OvfUpdateIntervalInMinutes','1','general');
Line 554: --The default network connectivity check timeout
Line 555: select 
fn_db_add_config_value('NetworkConnectivityCheckTimeoutInSeconds','120','general');
Line 556: -- AutoRecoveryConfiguration


....................................................
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()));
Solution for these divide to two methods
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 agreed will be done
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 109:      * returns a list of templates that are valid for update from the 
given templates list.
Line 110:      * valid template is a template which is not locked and none of 
it's disks is locked.
Line 111:      * @param templates
Line 112:      * @return
Line 113:      */
correct code
Line 114:     private List<VmTemplate> getTemplatesForUpdate(List<VmTemplate> 
templates) {
Line 115:         List<VmTemplate> toReturn = new ArrayList<VmTemplate>();
Line 116:         for (VmTemplate template : templates) {
Line 117:             if (VmTemplateStatus.Locked != template.getstatus()) {


Line 128:     /**
Line 129:      * returns true if none of the given disks is in status 'LOCKED', 
otherwise false.
Line 130:      * @param disks
Line 131:      * @return
Line 132:      */
correct code
Line 133:     private boolean verifyDisksNotLocked (List<DiskImage> disks) {
Line 134:         for (DiskImage disk : disks) {
Line 135:             if (disk.getimageStatus() == ImageStatus.LOCKED) {
Line 136:                 return false;


....................................................
File 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/UpdateVMVDSCommand.java
Line 45:             counter++;
Line 46:         }
Line 47:         return result;
Line 48:     }
Line 49: 
u should override ProceedProxyReturnValue();
Line 50:     @Override
Line 51:     protected VDSExceptionBase createDefaultConcreteException(String 
errorMessage) {
Line 52:         return new IrsOperationFailedNoFailoverException(errorMessage);
Line 53:     }


--
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