liron aravot has posted comments on this change.
Change subject: core:WIP: introducing OvfAutoUpdate
......................................................................
Patch Set 3: (10 inline comments)
Yair, i've replied your comments.
regarding the equals method, no - those fields are basically for use by the
ovfupdater and we will make use of them in import vm as well.
if you have a vm of which you update the description and later on you update it
back to it's original value, the db_generation of the vm has changed, but
equals should return true.
....................................................
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');
Line 3: UPDATE vm_static set ovf_generation = 1;
Line 4: ALTER TABLE vm_static ALTER COLUMN ovf_generation set default 0;
As this is an update script, at first i have to set 1 to all existing vms as
their ovf should be already updated.
later on i want to enforce that any newly added vm will have default value of 0
for the ovf_generation.
I separated this to two lines as I wanted this script to be clearer - instead
of defining the column with default 1 and then changing it to default 0 - it
would have seen weird and unclear in my opinion , do you prefer it that way?
....................................................
File backend/manager/dbscripts/upgrade/pre_upgrade/0000_config.sql
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');
Line 553: select
fn_db_add_config_value('OvfUpdateIntervalInMinutes','1','general');
didn't know that it's ordered, will change. thanks.
Line 554: --The default network connectivity check timeout
Line 555: select
fn_db_add_config_value('NetworkConnectivityCheckTimeoutInSeconds','120','general');
Line 556: -- AutoRecoveryConfiguration
Line 557: select fn_db_add_config_value('AutoRecoveryAllowedTypes','{\"storage
domains\":\"false\",\"hosts\":\"true\"}','general');
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InitBackendServicesOnStartupBean.java
Line 37: log.infoFormat("InitResourceManager: {0}", new Date());
Line 38: ResourceManager.getInstance().init();
Line 39: AsyncTaskManager.getInstance().InitAsyncTaskManager();
Line 40: log.infoFormat("AsyncTaskManager: {0}", new Date());
Line 41: OvfDataUpdater.getInstance().InitOvfDataUpdater();
Added it with the same convention as other classes here, will change.
Line 42: log.infoFormat("OvfDataUpdater: {0}", new Date());
Line 43:
Line 44: if (Config.<Boolean>
GetValue(ConfigValues.EnableVdsLoadBalancing)) {
Line 45: VdsLoadBalancer.EnableLoadBalancer();
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/OvfDataUpdater.java
Line 28: private static final Log log =
LogFactory.getLog(OvfDataUpdater.class);
Line 29: private static final OvfDataUpdater INSTANCE = new
OvfDataUpdater();
Line 30:
Line 31: private OvfDataUpdater(){
Line 32: SchedulerUtil scheduler =
SchedulerUtilQuartzImpl.getInstance();
I don't necessarily agree with you on that one, having somehow the instance
without the job is dangerous as the client won't have ovf updates without
knowing it - I don't want to be dependent on the call to the init method.
Line 33: scheduler.scheduleAFixedDelayJob(this, "ovfUpdate_timer", new
Class[] {},
Line 34: new Object[] {}, Config.<Integer>
GetValue(ConfigValues.OvfUpdateIntervalInMinutes),
Line 35: Config.<Integer>
GetValue(ConfigValues.OvfUpdateIntervalInMinutes), TimeUnit.SECONDS);
Line 36: }
Line 45:
Line 46: @OnTimerMethodAnnotation("ovfUpdate_timer")
Line 47: public void ovfUpdate_timer() {
Line 48: log.info("OvfDataUpdater: Attempting to update VMs/Templates
Ovf.");
Line 49: List<storage_pool> storagePools =
DbFacade.getInstance().getStoragePoolDao().getAll();
Will change.
Line 50: for (storage_pool pool : storagePools) {
Line 51: try {
Line 52: if (StoragePoolStatus.Up == pool.getstatus()) {
Line 53: log.infoFormat("OvfDataUpdater: Attempting to
update VMs/Templates Ovf in Data Center {0}",
Line 55: List<VM> vmsForUpdate =
Line 56: getVmsForUpdate(DbFacade.getInstance()
Line 57: .getVmDao()
Line 58:
.getAllVmsForOvfUpdateForStoragePool(pool.getId()));
Line 59: VmCommand.updateVmInSpm(pool.getId(),
vmsForUpdate);
It has been moved here, i've submitted it in a separate patch so it'll be
easier to review.
http://gerrit.ovirt.org/#/c/9340/
Line 60: if (!vmsForUpdate.isEmpty()) {
Line 61: for (VM vm : vmsForUpdate) {
Line 62: DbFacade.getInstance()
Line 63: .getVmDao()
Line 78: }
Line 79: } catch (Exception ex) {
Line 80: addAuditLogError(pool.getname());
Line 81: log.errorFormat("Exception while trying to update
VMs/Templates ovf in Data Center {0}, the exception is {1}",
Line 82: pool.getname(),
I agree, will add.
Line 83: ex.getMessage());
Line 84: }
Line 85: }
Line 86: }
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) {
I don't see any harm in it, will change if it bothers you :-)
Line 136: return false;
Line 137: }
Line 138: }
Line 139: return true;
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmCommand.java
Line 266: if (getVm() != null) {
Line 267: if (getVm().getstatus() == VMStatus.ImageLocked) {
Line 268: VmHandler.unlockVm(getVm(), getCompensationContext());
Line 269: }
Line 270: throw new RuntimeException("asd");
sorry, was here for debug purpose. not part of this patch.
Line 271: } else {
Line 272: setCommandShouldBeLogged(false);
Line 273: log.warn("VmCommand::EndVmCommand: Vm is null - not
performing EndAction on Vm");
Line 274: }
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmBase.java
Line 112: @Column(name = "db_generation")
Line 113: private long db_generation;
Line 114:
Line 115: @Column(name = "ovf_generation")
Line 116: private long ovf_generation;
I've added it in the convention of all other member for uniformity with all
other members of this class. I used long in order to support high number of
vm/template updates though i also believe that int max value will be never
reached.
Line 117:
Line 118: @Column(name = "is_smartcard_enabled")
Line 119: private boolean smartcardEnabled;
Line 120:
--
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: 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
