Michael Pasternak has posted comments on this change.
Change subject: restapi: add storage domain with existing conn id
......................................................................
Patch Set 1: I would prefer that you didn't submit this
(6 inline comments)
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendStorageDomainsResource.java
Line 74: public StorageDomainResource getStorageDomainSubResource(String
id) {
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) {
'StorageServerConnections' and 'connection', can't see the import of
StorageServerConnections, but if it's plural this is a collection of
connections?
Line 79: if (connection.getstorage_type().isFileDomain() &&
StringUtils.isEmpty(connection.getid())) {
Line 80:
connection.setid(addStorageServerConnection(connection, hostId));
Line 81: }
Line 82: entity.setStorage(connection.getid());
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()) ) {
who promise that storageConnectionFromUser != null? you have potential NPE
here, you should be checking storageConnectionFromUser != null before trying to
invoke storageConnectionFromUser.getId()
Line 173: validateParameters(storageDomain, "storage.type");
Line 174: cnx = mapToCnx(storageDomain);
Line 175: if(cnx.getstorage_type().isFileDomain()) {
Line 176: validateParameters(storageConnectionFromUser,
"path");
Line 176: validateParameters(storageConnectionFromUser,
"path");
Line 177: }
Line 178: }
Line 179: else {
Line 180: cnx =
getStorageServerConnection(storageConnectionFromUser.getId());
same, potential NPE
Line 181:
storageDomain.getStorage().setType(cnx.getstorage_type().toString());
Line 182: }
Line 183: StorageDomainStatic entity = mapToStatic(storageDomain);
Line 184: Response resp = null;
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()) ) {
same, potential NPE
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())) {
same, potential NPE
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())) {
same, potential NPE
Line 207: validateParameters(storageDomain.getStorage(),
"vfsType");
Line 208: }
Line 209: resp = addDomain(VdcActionType.AddGlusterFsStorageDomain,
storageDomain, entity, hostId, cnx);
Line 210: 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: 1
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
