Allon Mureinik 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
(5 inline comments)
If I understand correctly, it seems as though you moved some code around, and
forgot to add the files where you removed blocks.
Giving -1 just so this doesn't get merged by mistake.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStorageDomainCommon.java
Line 6: import org.ovirt.engine.core.common.errors.VdcBllMessages;
Line 7: import org.ovirt.engine.core.compat.Guid;
Line 8: import org.ovirt.engine.core.dal.dbbroker.DbFacade;
Line 9:
Line 10: import java.util.List;
This should be on the top
Line 11:
Line 12: public class AddStorageDomainCommon<T extends
StorageDomainManagementParameter> extends AddStorageDomainCommand<T> {
Line 13:
Line 14: /**
Line 39: }
Line 40: return true;
Line 41: }
Line 42:
Line 43: protected String
createDomainNamesListFromStorageDomains(List<StorageDomain> domains) {
Don't we have the same function in RemoveStorageServerConnectionCommand?
Shouldn't it be removed from there?
Line 44: // Build domain names list to display in the error
Line 45: StringBuilder domainNames = new StringBuilder();
Line 46: for (StorageDomain domain : domains) {
Line 47: domainNames.append(domain.getStorageName());
Line 51: domainNames.deleteCharAt(domainNames.length() - 1);
Line 52: return domainNames.toString();
Line 53: }
Line 54:
Line 55: protected List<StorageDomain> getStorageDomainsByConnId(String
connectionId) {
I think we have this in StorageServerConnectionCommandBase, no?
Line 56: return
getStorageDomainDAO().getAllByConnectionId(Guid.createGuidFromString(connectionId));
Line 57: }
Line 58:
Line 59: protected boolean prepareFailureMessageForDomains(String
domainNames) {
Line 55: protected List<StorageDomain> getStorageDomainsByConnId(String
connectionId) {
Line 56: return
getStorageDomainDAO().getAllByConnectionId(Guid.createGuidFromString(connectionId));
Line 57: }
Line 58:
Line 59: protected boolean prepareFailureMessageForDomains(String
domainNames) {
this too
Line 60: addCanDoActionMessage(String.format("$domainNames %1$s",
domainNames));
Line 61: return
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_CONNECTION_BELONGS_TO_SEVERAL_STORAGE_DOMAINS);
Line 62: }
Line 63:
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendStorageDomainsResource.java
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);
Don't all the file domains need a path?
what am I missing here?
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
