Moti Asayag has posted comments on this change.
Change subject: engine: Add command for removing external subnet
......................................................................
Patch Set 2:
(4 comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/network/RemoveSubnetFromProviderCommand.java
Line 28:
Line 29:
Line 30: private Provider<?> getProvider() {
Line 31: if (provider == null) {
Line 32: provider =
getDbFacade().getProviderDao().get(getSubnet().getExternalNetwork().getProviderId());
ExternalSubnet.getExternalNetwork() may return null as there are no specific
validation on it.
Either modify ExternalSubnet.ProviderNetwork not to null, or be verify it here
when attempting to extract the id (or any other valid solution that will
prevent the user from getting NPE).
Line 33: }
Line 34:
Line 35: return provider;
Line 36: }
Line 50: @Override
Line 51: protected boolean canDoAction() {
Line 52: ProviderValidator validator = new
ProviderValidator(getProvider());
Line 53:
Line 54: return validate(validator.providerIsSet()) &&
super.canDoAction();
what's the point in calling the super.canDoAction() if it always return true ?
Line 55: }
Line 56:
Line 57: @Override
Line 58: protected void executeCommand() {
Line 75: @Override
Line 76: public List<PermissionSubject> getPermissionCheckSubjects() {
Line 77: return Collections.singletonList(new
PermissionSubject(Guid.SYSTEM,
Line 78: VdcObjectType.System,
Line 79: ActionGroup.CREATE_STORAGE_POOL));
that seems a bit awkward action group for this action.
Line 80: }
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java
Line 601: REMOVE_VNIC_PROFILE_FAILED(1127),
Line 602: NETWORK_WITHOUT_INTERFACES(1128),
Line 603: VNIC_PROFILE_UNSUPPORTED_FEATURES(1129,
AuditLogTimeInterval.DAY.getValue()),
Line 604: SUBNET_REMOVED(1132),
Line 605: SUBNET_REMOVAL_FAILED(1133),
i think those are taken by:
LABEL_NETWORK(1132),
LABEL_NETWORK_FAILED(1133),
actually i used till 1139, but i guess it is going to be a dirty race till the
merge
(since our both patches depend on each other review :-P)
Line 606:
Line 607: // Import/Export
Line 608: IMPORTEXPORT_STARTING_EXPORT_VM(1162),
Line 609: IMPORTEXPORT_EXPORT_VM(1150),
--
To view, visit http://gerrit.ovirt.org/22686
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I948b8743ca73582cf8cf7dca3c02b8b983ee56ba
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Mike Kolesnik <[email protected]>
Gerrit-Reviewer: Moti Asayag <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches