Idan Shaby has posted comments on this change.

Change subject: core: WAD should be Ignored on File Domain Disks
......................................................................


Patch Set 2:

(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.
You are right.
In the first patch set I edited the code here and fixed it in the second set. 
But the formatting change remained.
Done.
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 sect
Done
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.
Done
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 th
Changed it to be initialized in a setUp() method.
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.
Done
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
Changed it to use the parameters data member.
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
Done
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
Done
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
Done
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
Done
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
Done
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

Reply via email to