Allon Mureinik has posted comments on this change.

Change subject: core: Add a validation when deactivate ISO domain.
......................................................................


Patch Set 14: Code-Review+1

(1 comment)

I agree with the performance concerns, but taking under consideration that this 
is an extremely rare operation, let's not block it.

It can be profiled and improved later.

http://gerrit.ovirt.org/#/c/20331/14/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommand.java:

Line 120:         if (!getParameters().getIsInternal() && 
getStorageDomain().getStorageDomainType() == StorageDomainType.ISO) {
Line 121:             List<String> vmNames = getVmsWithAttachedISO();
Line 122:             if (!vmNames.isEmpty()) {
Line 123:                 
failCanDoAction(VdcBllMessages.ERROR_CANNOT_DEACTIVATE_STORAGE_DOMAIN_WITH_ISO_ATTACHED,
Line 124:                         String.format("$VmNames %1$s", 
StringUtils.join(vmNames, ",")));
The alignment looks funky - please make sure it's just a gerrit foobar
Line 125:             }
Line 126:         }
Line 127:         if (!getParameters().getIsInternal()
Line 128:                 && !getVmDAO()


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I47c1a8155762ecd0b04bb17676151946982bb919
Gerrit-PatchSet: 14
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Cheryn Tan <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Itamar Heim <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Sergey Gotliv <[email protected]>
Gerrit-Reviewer: Tal Nisan <[email protected]>
Gerrit-Reviewer: Vitor de Lima <[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

Reply via email to