Martin Betak has posted comments on this change.

Change subject: engine: Enable per-VM configuration of migration downtime
......................................................................


Patch Set 5:

(5 comments)

http://gerrit.ovirt.org/#/c/22272/5/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmCommand.java:

Line 171:                 dstVdsHost, MigrationMethod.ONLINE, 
isTunnelMigrationUsed(), getMigrationNetworkIp(), 
getVds().getVdsGroupCompatibilityVersion(),
Line 172:                 getMaximumMigrationDowntime());
Line 173:     }
Line 174: 
Line 175:     private Integer getMaximumMigrationDowntime() {
> looks like this method can return int, no?
In case we are guaranteed Config.getValue will never return null, I suppose yes.
Line 176:         if (getVm().getMigrationDowntime() != null) {
Line 177:             return getVm().getMigrationDowntime();
Line 178:         }
Line 179: 


http://gerrit.ovirt.org/#/c/22272/5/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmBase.java
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmBase.java:

Line 187:      * value of null indicates that the VDSM config value will be used
Line 188:      */
Line 189:     @EditableField
Line 190:     @Min(value = 0, message = 
"VALIDATION.VM.MIGRATION_DOWNTIME_RANGE")
Line 191:     private Integer migrationDowntime;
> please add it to the constructor
Done
Line 192: 
Line 193:     public VmBase() {
Line 194:         name = "";
Line 195:         interfaces = new ArrayList<VmNetworkInterface>();


http://gerrit.ovirt.org/#/c/22272/5/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/ConfigurationValues.java
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/ConfigurationValues.java:

Line 112:     SpeedOptimizationSchedulingThreshold,
Line 113:     SchedulerAllowOverBooking,
Line 114:     SchedulerOverBookingThreshold,
Line 115:     UserSessionTimeOutInterval(ConfigAuthType.User),
Line 116:     DefaultMaximumMigrationDowntime;
> this is in use in the frontend?
yes, in the tooltip we display to the user the current default
Line 117: 
Line 118:     public static enum ConfigAuthType {
Line 119:         Admin,
Line 120:         User


http://gerrit.ovirt.org/#/c/22272/5/backend/manager/modules/dal/src/test/resources/fixtures.xml
File backend/manager/modules/dal/src/test/resources/fixtures.xml:

Line 1524:         <column>image_type_id</column>
Line 1525:         <column>sso_method</column>
Line 1526:         <column>original_template_id</column>
Line 1527:         <column>original_template_name</column>
Line 1528:         <column>migration_downtime</column>
> please put at least one non null value.. :)
Done
Line 1529:         <!-- Templates -->
Line 1530:         <row>
Line 1531:             <value>00000000-0000-0000-0000-000000000000</value>
Line 1532:             <value>TEMPLATE</value>


http://gerrit.ovirt.org/#/c/22272/5/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/MigrateBrokerVDSCommand.java
File 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/MigrateBrokerVDSCommand.java:

Line 34:             migrationInfo.put("abortOnError", Boolean.TRUE.toString());
Line 35:         }
Line 36: 
Line 37:         if (parameters.getMigrationDowntime() != null && 
parameters.getMigrationDowntime() != 0) {
Line 38:             migrationInfo.put(VdsProperties.MIGRATION_DOWNTIME, 
parameters.getMigrationDowntime().toString());
> no need to block sending this to older vdsms? (by compatibility)
by looking at git blame this parameter has been in VDSM since the start of 
oVirt project :-)
Line 39:         }
Line 40:     }
Line 41: 
Line 42:     @Override


-- 
To view, visit http://gerrit.ovirt.org/22272
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia16f66540497d20178efae6989710000110a81d6
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Martin Betak <mbe...@redhat.com>
Gerrit-Reviewer: Martin Betak <mbe...@redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skriva...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Roy Golan <rgo...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to