Alissa Bonas has posted comments on this change.

Change subject: restapi: add storage server connections resource
......................................................................


Patch Set 22: (8 inline comments)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/GetStorageServerConnectionByIdQuery.java
Line 13:     @Override
Line 14:     protected void executeQueryCommand() {
Line 15:         getQueryReturnValue().setReturnValue(DbFacade.getInstance().
Line 16:                 getStorageServerConnectionDao().get(getParameters().
Line 17:                           getServerConnectionId()));
let's leave it, it's already here :)
Line 18:     }


....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RemoveStorageServerConnectionCommand.java
Line 40:             return 
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_CONNECTION_NOT_EXIST);
Line 41:         }
Line 42:         // if user passed only the connection id for removal, vdsm 
still needs few more details in order to disconnect, so
Line 43:         // bringing them from db and repopulating them in the 
connection object received in input parameters
Line 44:         populateMissingFields(connection);
connect/disconnect actions done on vdsm side.
in the remove case, the disconnect is relevant of course.
Check BuildStructFromConnectionListObject method in 
ConnectStorageServerVDSCommand
Line 45:         StorageType storageType = connection.getstorage_type();
Line 46:         if (storageType.isFileDomain()) {
Line 47:             // go to storage domain static, get all storage domains 
where storage field  = storage connection id
Line 48:            domains = getStorageDomainsByConnId(connectionId);


....................................................
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/StorageServerConnectionQueryParametersBase.java
Line 18:     public StorageServerConnectionQueryParametersBase(String 
serverConnectionId) {
Line 19:         setServerConnectionId(serverConnectionId);
Line 20:     }
Line 21: 
Line 22:     //used by REST because AbstractBackendResource has id member that 
is always assumed to be guid
Done
Line 23:     public StorageServerConnectionQueryParametersBase(Guid 
serverConnectionId) {
Line 24:         this(serverConnectionId.toString());
Line 25:     }
Line 26: 


....................................................
File 
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StorageServerConnectionDAODbFacadeImpl.java
Line 22:         StorageServerConnectionDAO {
Line 23: 
Line 24:     @Override
Line 25:     public List<StorageServerConnections> getAll() {
Line 26:         MapSqlParameterSource parameterSource = 
getCustomMapSqlParameterSource();
I really don't like inlining. debugging nightmare...
Line 27:         return 
getCallsHandler().executeReadList("GetAllstorage_server_connections", 
mapper,parameterSource) ;
Line 28:     }
Line 29: 
Line 30:     @Override


....................................................
File 
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendStorageServerConnectionResource.java
Line 28:     }
Line 29: 
Line 30:     @Override
Line 31:     public Storage update(Storage connection) {
Line 32:            validateParameters(connection, "address", "type", "host");
Not exactly.
For file domains, the connection field "formula" in bll is converting from REST 
like this:
connection.getAddress()+":"+connection.getPath()
Line 33:            validateEnums(Storage.class, connection);
Line 34:            return performUpdate(connection,
Line 35:                              new 
QueryIdResolver<String>(VdcQueryType.GetStorageServerConnectionById, 
StorageServerConnectionQueryParametersBase.class),
Line 36:                              
VdcActionType.UpdateStorageServerConnection,


Line 57:         }
Line 58: 
Line 59:          private Guid getHostId(Host host) {
Line 60:         // presence of host ID or name already validated
Line 61:         return host.isSetId()
Done
Line 62:                ? new Guid(host.getId())
Line 63:                : host.isSetName()
Line 64:                  ? getEntity(VDS.class,
Line 65:                              SearchType.VDS,


....................................................
File 
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendStorageServerConnectionsResource.java
Line 65:             break;
Line 66:         case NFS:
Line 67:             validateParameters(storage, "path");
Line 68:             break;
Line 69:         case LOCALFS:
Done
Line 70:             validateParameters(storage, "path");
Line 71:             break;
Line 72:         case POSIXFS:
Line 73:         case GLUSTERFS:


....................................................
File 
backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/StorageDomainMapper.java
Line 12: import org.ovirt.engine.api.restapi.model.StorageFormat;
Line 13: import org.ovirt.engine.api.restapi.utils.GuidUtils;
Line 14: import 
org.ovirt.engine.core.common.businessentities.StorageDomainStatic;
Line 15: import 
org.ovirt.engine.core.common.businessentities.StorageServerConnections;
Line 16: 
yes you're right, but it's already here, so let's leave it. will try better in 
future code.
Line 17: public class StorageDomainMapper {
Line 18: 
Line 19:     @Mapping(from = StorageDomain.class, to = 
StorageDomainStatic.class)
Line 20:     public static StorageDomainStatic map(StorageDomain model, 
StorageDomainStatic template) {


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If6bc32ead098390723825872f6fb292097d52835
Gerrit-PatchSet: 22
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: Daniel Erez <[email protected]>
Gerrit-Reviewer: Michael Pasternak <[email protected]>
Gerrit-Reviewer: Tal Nisan <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to