Michael Pasternak has posted comments on this change.
Change subject: restapi: add storage domain with existing conn id
......................................................................
Patch Set 2: I would prefer that you didn't submit this
(7 inline comments)
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendStorageDomainsResource.java
Line 75: return inject(new BackendStorageDomainResource(id, this));
Line 76: }
Line 77:
Line 78: private Response addDomain(VdcActionType action, StorageDomain
model, StorageDomainStatic entity, Guid hostId, StorageServerConnections
connection) {
Line 79: if (connection.getstorage_type().isFileDomain() &&
StringUtils.isEmpty(connection.getid())) {
please use .isSetX() instead of StringUtils
Line 80:
connection.setid(addStorageServerConnection(connection, hostId));
Line 81: }
Line 82: entity.setStorage(connection.getid());
Line 83: if (action == VdcActionType.AddNFSStorageDomain) {
Line 168: validateEnums(StorageDomain.class, storageDomain);
Line 169: Guid hostId = getHostId(storageDomain);
Line 170: StorageServerConnections cnx = null;
Line 171:
Line 172: if (StringUtils.isEmpty(storageConnectionFromUser.getId()) ) {
please do: !storageConnectionFromUser.isSetId()
Line 173: validateParameters(storageDomain, "storage.type");
Line 174: cnx = mapToCnx(storageDomain);
Line 175: if(cnx.getstorage_type().isFileDomain()) {
Line 176: validateParameters(storageConnectionFromUser,
"path");
Line 177: }
Line 178: }
Line 179: else {
Line 180: cnx =
getStorageServerConnection(storageConnectionFromUser.getId());
Line 181:
storageDomain.getStorage().setType(cnx.getstorage_type().toString());
you should be using mapper here to set public enum value and not backend enum
value,
cause later in mapToStatic() "type" converted from public->backend enum value
Line 182: }
Line 183: StorageDomainStatic entity = mapToStatic(storageDomain);
Line 184: Response resp = null;
Line 185: switch (entity.getStorageType()) {
Line 187: case FCP:
Line 188: resp = addSAN(storageDomain, entity.getStorageType(),
entity, hostId);
Line 189: break;
Line 190: case NFS:
Line 191: if
(StringUtils.isEmpty(storageConnectionFromUser.getId()) ) {
please use .isSetX() instead of StringUtils
Line 192: validateParameters(storageDomain.getStorage(),
"address");
Line 193: }
Line 194: resp = addDomain(VdcActionType.AddNFSStorageDomain,
storageDomain, entity, hostId, cnx);
Line 195: break;
Line 196: case LOCALFS:
Line 197: resp = addDomain(VdcActionType.AddLocalStorageDomain,
storageDomain, entity, hostId, cnx);
Line 198: break;
Line 199: case POSIXFS:
Line 200: if
(StringUtils.isEmpty(storageConnectionFromUser.getId())) {
please use .isSetX() instead of StringUtils
Line 201: validateParameters(storageDomain.getStorage(),
"vfsType");
Line 202: }
Line 203: resp = addDomain(VdcActionType.AddPosixFsStorageDomain,
storageDomain, entity, hostId, cnx);
Line 204: break;
Line 202: }
Line 203: resp = addDomain(VdcActionType.AddPosixFsStorageDomain,
storageDomain, entity, hostId, cnx);
Line 204: break;
Line 205: case GLUSTERFS:
Line 206: if
(StringUtils.isEmpty(storageConnectionFromUser.getId())) {
please use .isSetX() instead of StringUtils
Line 207: validateParameters(storageDomain.getStorage(),
"vfsType");
Line 208: }
Line 209: resp = addDomain(VdcActionType.AddGlusterFsStorageDomain,
storageDomain, entity, hostId, cnx);
Line 210: break;
Line 205: case GLUSTERFS:
Line 206: if
(StringUtils.isEmpty(storageConnectionFromUser.getId())) {
Line 207: validateParameters(storageDomain.getStorage(),
"vfsType");
Line 208: }
Line 209: resp = addDomain(VdcActionType.AddGlusterFsStorageDomain,
storageDomain, entity, hostId, cnx);
it's already validated at: validateParameters(storageConnectionFromUser,
"path");
Line 210: break;
Line 211:
Line 212: default:
Line 213: break;
--
To view, visit http://gerrit.ovirt.org/17177
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Icb0495be711f80d71ad5334080228faee03d03dc
Gerrit-PatchSet: 2
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: Ori Liel <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
