Alissa Bonas has posted comments on this change.

Change subject: core: fix redundant storage server conn in db
......................................................................


Patch Set 3: (3 inline comments)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DisconnectStorageServerConnectionCommand.java
Line 18:         super(parameters);
Line 19:     }
Line 20: 
Line 21:     @Override
Line 22:     protected void executeCommand() {
I don't understand the purpose of this comment.
Line 23:         setSucceeded(disconnectStorage());
Line 24:     }
Line 25: 
Line 26:     @Override


Line 38: 
Line 39:         return returnValue;
Line 40:     }
Line 41: 
Line 42:     protected boolean disconnectStorage() {
I simply extracted here a code from executeCommand to a separate method so it 
can be reused by inheriting command which is the focus of this patch, without 
changing any functionality.
If you think a general refactoring is in place in this class to change it to 
use runVdsCommand method, it might be a good idea, however it should be done 
separately in a a different patch as it might change the way this command 
behaves and it is out of scope here. (it might have a different error handling, 
has different signature, etc.)
Line 43:         return Backend.getInstance()
Line 44:                .getResourceManager()
Line 45:                .RunVdsCommand(
Line 46:                     VDSCommandType.DisconnectStorageServer,


Line 45:                .RunVdsCommand(
Line 46:                     VDSCommandType.DisconnectStorageServer,
Line 47:                         new 
ConnectStorageServerVDSCommandParameters(getParameters().getVdsId(), 
getParameters()
Line 48:                                 .getStoragePoolId(), 
getParameters().getStorageServerConnection().getstorage_type(),
Line 49:                                 new 
java.util.ArrayList<StorageServerConnections>(java.util.Arrays
Good idea - for a separate patch.
Line 50:                                         .asList(new 
StorageServerConnections[] { getConnection() })))).getSucceeded() ;
Line 51:     }
Line 52: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iea5468371514bd2c7bc043a6c5520e2864a09fe8
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alissa Bonas <[email protected]>
Gerrit-Reviewer: Alissa Bonas <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Michael Kublin <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to