Liron Aravot has posted comments on this change.
Change subject: core: add ability edit nfs path in webadmin
......................................................................
Patch Set 2: (7 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommand.java
Line 14: import java.util.ArrayList;
Line 15: import java.util.Arrays;
Line 16: import java.util.Collections;
Line 17: import java.util.Map;
Line 18:
because we don't want to hold open transaction through long time (we have vdsm
calls here which each can take few mins) as we are limited in possible open
transaction count. furthermore - the transaction here has no need as we have
only one db call so it's better to be performed as local db transaction rather
then global transaction (better performance and resources utilization).
Line 19: public class UpdateStorageServerConnectionCommand<T extends
StorageServerConnectionParametersBase> extends
StorageServerConnectionCommandBase<T> {
Line 20: StorageServerConnections oldConnection = null;
Line 21:
Line 22: public UpdateStorageServerConnectionCommand(T parameters) {
Line 44: protected void executeCommand() {
Line 45: ConnectStorageServerVDSCommandParameters
newConnectionParametersForVdsm = new
ConnectStorageServerVDSCommandParameters(getParameters().getVdsId(),
getParameters()
Line 46: .getStoragePoolId(),
getParameters().getStorageServerConnection().getstorage_type(),
Line 47: new
ArrayList<StorageServerConnections>(Arrays
Line 48:
.asList(getParameters().getStorageServerConnection())));
we create here a list backed by the array and then arraylist with it's elemets,
so it's less operations..furthermore, in my opinion, Collections.singletonList
is "cleaner" :)
Line 49:
Line 50: VDSReturnValue returnValueConnect =
runVdsCommand(VDSCommandType.ConnectStorageServer,newConnectionParametersForVdsm);
Line 51: if (returnValueConnect.getSucceeded()) {
Line 52:
getDbFacade().getStorageServerConnectionDao().update(getParameters().getStorageServerConnection());
Line 53: //Disconnect the old storage server connection via vdsm -
only after update in db to a new connection succeeds
Line 54: ConnectStorageServerVDSCommandParameters
oldConnectionParametersForVdsm = new
ConnectStorageServerVDSCommandParameters(getParameters().getVdsId(),
getParameters()
Line 55: .getStoragePoolId(),
oldConnection.getstorage_type(),
Line 56: new
ArrayList<StorageServerConnections>(Arrays
Line 57: .asList(oldConnection)));
same answer
Line 58:
runVdsCommand(VDSCommandType.DisconnectStorageServer,oldConnectionParametersForVdsm);
Line 59: setSucceeded(true);
Line 60: }
Line 61: }
Line 54: ConnectStorageServerVDSCommandParameters
oldConnectionParametersForVdsm = new
ConnectStorageServerVDSCommandParameters(getParameters().getVdsId(),
getParameters()
Line 55: .getStoragePoolId(),
oldConnection.getstorage_type(),
Line 56: new
ArrayList<StorageServerConnections>(Arrays
Line 57: .asList(oldConnection)));
Line 58:
runVdsCommand(VDSCommandType.DisconnectStorageServer,oldConnectionParametersForVdsm);
if we are in a pool, then yes..
regardless, i'd suggest to have this on a separate thread/threads - so the
command will end and the lock will be released as soon as we perform the
"needed" operations (connect and db update).
Line 59: setSucceeded(true);
Line 60: }
Line 61: }
Line 62:
Line 55: .getStoragePoolId(),
oldConnection.getstorage_type(),
Line 56: new
ArrayList<StorageServerConnections>(Arrays
Line 57: .asList(oldConnection)));
Line 58:
runVdsCommand(VDSCommandType.DisconnectStorageServer,oldConnectionParametersForVdsm);
Line 59: setSucceeded(true);
to the success of the connect and the db update
Line 60: }
Line 61: }
Line 62:
Line 63: @Override
Line 56: new
ArrayList<StorageServerConnections>(Arrays
Line 57: .asList(oldConnection)));
Line 58:
runVdsCommand(VDSCommandType.DisconnectStorageServer,oldConnectionParametersForVdsm);
Line 59: setSucceeded(true);
Line 60: }
repointing an existing one is basically adding an existing one except to the db
operations - I'm saying that the same operations should be done after
performing this (see what's done there..the domain data is updated, free
space..etc), why would the free space and such shouldn't be updated if we
changed location?
Line 61: }
Line 62:
Line 63: @Override
Line 64: protected Map<String, Pair<String, String>> getExclusiveLocks() {
Line 61: }
Line 62:
Line 63: @Override
Line 64: protected Map<String, Pair<String, String>> getExclusiveLocks() {
Line 65: return
Collections.singletonMap(getParameters().getStorageServerConnection().getid(),
LockMessagesMatchUtil.STORAGE);
the operation is related to the domain, we change domain location - we want to
prevent other operations on it meanwhile.
Line 66: }
Line 67:
Line 68: @Override
Line 69: protected void setActionMessageParameters() {
--
To view, visit http://gerrit.ovirt.org/12372
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifaff5344ff191d6bdf53cc706c7bb796167a56b3
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alissa Bonas <abo...@redhat.com>
Gerrit-Reviewer: Alissa Bonas <abo...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Liron Aravot <lara...@redhat.com>
Gerrit-Reviewer: Tal Nisan <tni...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
