Allon Mureinik has posted comments on this change. Change subject: core: WAD should be Ignored on File Domain Disks ......................................................................
Patch Set 2: Code-Review-1 (11 comments) http://gerrit.ovirt.org/#/c/31687/2/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/DestroyImageCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/DestroyImageCommand.java: Line 37: @Override Line 38: protected void executeCommand() { Line 39: Guid taskId = persistAsyncTaskPlaceHolder(VdcActionType.DestroyImage); Line 40: Line 41: VDSReturnValue vdsReturnValue = runVdsCommand(VDSCommandType.DestroyImage, createVDSParameters()); This formatting change is unrelated to the patch. Please remove it (or submit it in a separate patch if it's important to you) Line 42: Line 43: if (vdsReturnValue != null && vdsReturnValue.getCreationInfo() != null) { Line 44: getParameters().setVdsmTaskIds(new ArrayList<Guid>()); Line 45: Guid result = createTask(taskId, vdsReturnValue.getCreationInfo(), VdcActionType.DestroyImage, http://gerrit.ovirt.org/#/c/31687/2/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/SecureDeletionHandlerTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/SecureDeletionHandlerTest.java: Line 7: import org.ovirt.engine.core.common.vdscommands.StorageDomainIdParametersBase; Line 8: Line 9: import java.text.MessageFormat; Line 10: Line 11: import static org.junit.Assert.assertEquals; according to the project's style static imports should be in the first section, not the last - please reconfigure your IDE accordingly and move this insert. Line 12: Line 13: @RunWith(MockitoJUnitRunner.class) Line 14: public class SecureDeletionHandlerTest { Line 15: Line 9: import java.text.MessageFormat; Line 10: Line 11: import static org.junit.Assert.assertEquals; Line 12: Line 13: @RunWith(MockitoJUnitRunner.class) You aren't mocking anything - you do not need this runner. Line 14: public class SecureDeletionHandlerTest { Line 15: Line 16: private ParametersWithSecureDeletion parameters; Line 17: Line 12: Line 13: @RunWith(MockitoJUnitRunner.class) Line 14: public class SecureDeletionHandlerTest { Line 15: Line 16: private ParametersWithSecureDeletion parameters; If you're not initializing this object in the ctor or the @Before method there's no point in having it as a data member - please convert it to a local variable. Line 17: Line 18: @Test Line 19: public void parametersWithSecureDeletionAreFixedOnFileDomainWhenPostZeroIsTrue() { Line 20: parameters = new ParametersWithSecureDeletion(true); Line 34: // On file domain. Line 35: assertPostZeroValue(parameters, true, false); Line 36: Line 37: // On block domain. Line 38: assertPostZeroValue(parameters, false, false); This should be separated to two @Test methods. Line 39: } Line 40: Line 41: private void assertPostZeroValue(ParametersWithSecureDeletion parameters, boolean isFileDomain, Line 42: boolean postZeroExpectedValue) { Line 37: // On block domain. Line 38: assertPostZeroValue(parameters, false, false); Line 39: } Line 40: Line 41: private void assertPostZeroValue(ParametersWithSecureDeletion parameters, boolean isFileDomain, should be static Line 42: boolean postZeroExpectedValue) { Line 43: SecureDeletionHandler.fixParametersWithSecureDeletion(parameters, isFileDomain); Line 44: assertEquals(MessageFormat.format( Line 45: "Wrong VDS command parameters: 'postZero' should be {0}.", postZeroExpectedValue), http://gerrit.ovirt.org/#/c/31687/2/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/CopyImageVDSCommandParameters.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/CopyImageVDSCommandParameters.java: Line 108: Line 109: public boolean getPostZero() { Line 110: return privatePostZero; Line 111: } Line 112: Missing @Override Line 113: public void setPostZero(boolean postZero) { Line 114: privatePostZero = postZero; Line 115: } Line 116: http://gerrit.ovirt.org/#/c/31687/2/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/DeleteImageGroupVDSCommandParameters.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/DeleteImageGroupVDSCommandParameters.java: Line 18: Line 19: public boolean getPostZero() { Line 20: return postZero; Line 21: } Line 22: Missing @Override Line 23: public void setPostZero(boolean postZero) { Line 24: this.postZero = postZero; Line 25: } Line 26: http://gerrit.ovirt.org/#/c/31687/2/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/DestroyImageVDSCommandParameters.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/DestroyImageVDSCommandParameters.java: Line 28: Line 29: public boolean getPostZero() { Line 30: return privatePostZero; Line 31: } Line 32: Missing @Override Line 33: public void setPostZero(boolean postZero) { Line 34: privatePostZero = postZero; Line 35: } Line 36: http://gerrit.ovirt.org/#/c/31687/2/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/MergeSnapshotsVDSCommandParameters.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/MergeSnapshotsVDSCommandParameters.java: Line 32: Line 33: public boolean getPostZero() { Line 34: return privatePostZero; Line 35: } Line 36: Missing @Override Line 37: public void setPostZero(boolean postZero) { Line 38: privatePostZero = postZero; Line 39: } Line 40: http://gerrit.ovirt.org/#/c/31687/2/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/MoveImageGroupVDSCommandParameters.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/MoveImageGroupVDSCommandParameters.java: Line 38: Line 39: public boolean getPostZero() { Line 40: return privatePostZero; Line 41: } Line 42: Missing @Override Line 43: public void setPostZero(boolean postZero) { Line 44: privatePostZero = postZero; Line 45: } Line 46: -- To view, visit http://gerrit.ovirt.org/31687 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0df2c07282994557e149db922cc1d3a166c5aa8f Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Idan Shaby <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Idan Shaby <[email protected]> Gerrit-Reviewer: Liron Aravot <[email protected]> Gerrit-Reviewer: Tal Nisan <[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
