Allon Mureinik has posted comments on this change.
Change subject: core, engine: Prevent snapshot in ppc64
......................................................................
Patch Set 2: Code-Review-1
(6 comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java
Line 402
Line 403
Line 404
Line 405
Line 406
Remove this blank line is unrelated to the patch.
Line 268: }
Line 269:
Line 270: if (getParameters().isSaveMemory()
Line 271: && !isMemorySnapshotSupported()) {
Line 272: log.warnFormat("A memory snapshot was required but not
created, since VM architecture does not support it.");
This line will never be executed - the first if statement in the method will
evaluate to true, and a NullableMemoryImageBuilder will be returned.
Moreover, in such a situation I don't think we should just log the discrepancy,
but fail canDoAction() - the use has obviously requested an impossible command
to be run.
Line 273: }
Line 274:
Line 275: incrementVmGeneration();
Line 276:
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java
Line 136: if (getVm().getStatus() != VMStatus.Suspended) {
Line 137: boolean archSupportSnapshot =
FeatureSupported.isMemorySnapshotSupportedByArchitecture(
Line 138: getVm().getClusterArch(),
Line 139: getVm().getVdsGroupCompatibilityVersion());
Line 140: memorySnapshotSupported =
FeatureSupported.memorySnapshot(getVm().getVdsGroupCompatibilityVersion()) &&
archSupportSnapshot;
Minor performance enhancement - flip the && operands' order.
This way, if archSupportSnapshot is false, you won't need to execute another
costly FeatureSupported call
Line 141: // If the VM is not hibernated, save the hibernation
volume from the baseline snapshot
Line 142: memoryVolumeFromSnapshot =
getActiveSnapshot().getMemoryVolume();
Line 143: }
Line 144: }
....................................................
File
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmCommandTest.java
Line 3
Line 4
Line 5
Line 6
Line 7
Is this import removal intentional?
Line 28
Line 29
Line 30
Line 31
Line 32
Is this import removal intentional?
....................................................
File packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql
Line 672: select fn_db_add_config_value('IsSnapshotSupported','{"undefined":
"true", "x86_64": "true", "ppc64" : "false" }','3.0');
Line 673: select fn_db_add_config_value('IsSnapshotSupported','{"undefined":
"true", "x86_64": "true", "ppc64" : "false" }','3.1');
Line 674: select fn_db_add_config_value('IsSnapshotSupported','{"undefined":
"true", "x86_64": "true", "ppc64" : "false" }','3.2');
Line 675: select fn_db_add_config_value('IsSnapshotSupported','{"undefined":
"true", "x86_64": "true", "ppc64" : "false" }','3.3');
Line 676:
What's the merit of separating this configuration to versions? Why not just use
"general"?
Line 677:
------------------------------------------------------------------------------------
Line 678: -- Update with override section
Line 679:
------------------------------------------------------------------------------------
Line 680:
--
To view, visit http://gerrit.ovirt.org/21657
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I4f201b63ddf441a9bc76a37fda33f54fa766d937
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Leonardo Bianconi <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Arik Hadas <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Gustavo Frederico Temple Pedrosa
<[email protected]>
Gerrit-Reviewer: Leonardo Bianconi <[email protected]>
Gerrit-Reviewer: Liron Ar <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Roy Golan <[email protected]>
Gerrit-Reviewer: Vitor de Lima <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: liron aravot <[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
