Allon Mureinik has posted comments on this change.

Change subject: db, core, webadmin: Add is_live_merge_supported flag
......................................................................


Patch Set 1: Code-Review-1

(5 comments)

Possible NPE in VDS - see inline.
Also a couple of nits throughout the patch.

http://gerrit.ovirt.org/#/c/30773/1/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDS.java
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDS.java:

Line 1404:     public Boolean getLiveSnapshotSupport() {
Line 1405:         return this.mVdsDynamic.getLiveSnapshotSupport();
Line 1406:     }
Line 1407: 
Line 1408:     public void setLiveMergeSupport(Boolean value) {
VdsDynamic uses a primitive boolean  - should be the same here.
Line 1409:         this.mVdsDynamic.setLiveMergeSupport(value);
Line 1410:     }
Line 1411: 
Line 1412:     public Boolean getLiveMergeSupport() {


Line 1408:     public void setLiveMergeSupport(Boolean value) {
Line 1409:         this.mVdsDynamic.setLiveMergeSupport(value);
Line 1410:     }
Line 1411: 
Line 1412:     public Boolean getLiveMergeSupport() {
VdsDynamic uses a primitive boolean  - should be the same here.
Line 1413:         return this.mVdsDynamic.getLiveMergeSupport();
Line 1414:     }


http://gerrit.ovirt.org/#/c/30773/1/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VdsDynamic.java
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VdsDynamic.java:

Line 201:         numaNodeList = new ArrayList<VdsNumaNode>();
Line 202:         autoNumaBalancing = AutoNumaBalanceStatus.UNKNOWN;
Line 203:         supportedRngSources = new HashSet<VmRngDevice.Source>();
Line 204:         liveSnapshotSupport = true;  // usually supported, 
exceptional case if it isn't.
Line 205:         liveMergeSupport = true;
Not sure about this default.
Then again, probably better to align to liveSnapshotSupport
Line 206:     }
Line 207: 
Line 208:     public Integer getcpu_cores() {
Line 209:         return this.cpu_cores;


http://gerrit.ovirt.org/#/c/30773/1/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmSnapshotListModel.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmSnapshotListModel.java:

Line 782:             setLiveMergeSupported(false);
Line 783:             return;
Line 784:         }
Line 785: 
Line 786:         // TODO GP
que?
Line 787:         AsyncQuery query = new AsyncQuery(this, new 
INewAsyncCallback() {
Line 788:             @Override
Line 789:             public void onSuccess(Object model, Object returnValue) {
Line 790:                 VmSnapshotListModel target = (VmSnapshotListModel) 
model;


http://gerrit.ovirt.org/#/c/30773/1/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/VdsmErrors.properties
File 
frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/VdsmErrors.properties:

Line 323: PROVIDER_FAILURE=Failed to communicate with the external provider.
Line 324: PROVIDER_IMPORT_CERTIFICATE_CHAIN_ERROR=Failed to import provider 
certificate chain.
Line 325: PROVIDER_SSL_FAILURE=SSL problem while trying to connect to the 
external provider.
Line 326: FAILED_UPDATE_RUNNING_VM=Failed to update VM while it is running, 
please try again when the VM is Down.
Line 327: VM_NOT_QUALIFIED_FOR_SNAPSHOT_MERGE=To merge snapshots, a VM must be 
Down, Up or Paused. If running, the host must also be capable of live merging 
snapshots.
Wouldn't we like to give two distinct error messages?
Line 328: MIGRATION_DEST_INVALID_HOSTNAME=Migration destination has an invalid 
hostname
Line 329: MIGRATION_CANCEL_ERROR=Migration not in progress
Line 330: DB=Database error.
Line 331: MAC_POOL_NO_MACS_LEFT=No available MAC addresses left in the MAC 
Address Pool.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I932a74f4010b8b256f8c0cd27a2279b9d83efbb9
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Greg Padgett <[email protected]>
Gerrit-Reviewer: Adam Litke <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Greg Padgett <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to