Allon Mureinik has posted comments on this change.
Change subject: core: Add a validation when deactivate ISO domain.
......................................................................
Patch Set 4: Code-Review-1
(5 comments)
See inline.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommand.java
Line 127:
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_DETECTED_ACTIVE_VMS);
Line 128: return false;
Line 129: }
Line 130: if (getStorageDomain().getStorageDomainType() ==
StorageDomainType.ISO) {
Line 131: addWarningForVmsWithAttachedDisk();
we should return false here.
Line 132: }
Line 133: if (getStoragePool().getspm_vds_id() != null) {
Line 134: // In case there are running tasks in the pool, it is
impossible to deactivate the master storage domain
Line 135: if (getStorageDomain().getStorageDomainType() ==
StorageDomainType.Master &&
Line 375: }
Line 376: return null;
Line 377: }
Line 378:
Line 379: private static boolean isVMHasAttachedDiskOrFloppy(Guid vmId) {
why static? this will interfere with future developers writing tests for this
flow.
Line 380: List<VmDevice> cdList =
Line 381: DbFacade.getInstance()
Line 382: .getVmDeviceDao()
Line 383: .getVmDeviceByVmIdTypeAndDevice(vmId,
Line 378:
Line 379: private static boolean isVMHasAttachedDiskOrFloppy(Guid vmId) {
Line 380: List<VmDevice> cdList =
Line 381: DbFacade.getInstance()
Line 382: .getVmDeviceDao()
Use getVmDeviceDao() once the method is no longer static.
Line 383: .getVmDeviceByVmIdTypeAndDevice(vmId,
Line 384: VmDeviceGeneralType.DISK,
Line 385: VmDeviceType.CDROM.getName());
Line 386: for (VmDevice cdDevice : cdList) {
Line 382: .getVmDeviceDao()
Line 383: .getVmDeviceByVmIdTypeAndDevice(vmId,
Line 384: VmDeviceGeneralType.DISK,
Line 385: VmDeviceType.CDROM.getName());
Line 386: for (VmDevice cdDevice : cdList) {
This entire loop is the same for CD and Floppy devices - extract it to a method.
Or even better - make a single DAO call of getVmDeviceByVmIdAndType(vmId,
VmDeviceGeneralType.DISK), and filter CDs and floppies when you loop over it.
Line 387: String path = (String)
cdDevice.getSpecParams().get("path");
Line 388: if (path != null && path != "") {
Line 389: return false;
Line 390: }
Line 384: VmDeviceGeneralType.DISK,
Line 385: VmDeviceType.CDROM.getName());
Line 386: for (VmDevice cdDevice : cdList) {
Line 387: String path = (String)
cdDevice.getSpecParams().get("path");
Line 388: if (path != null && path != "") {
use if(!StringUtils.isEmpty(path))
Line 389: return false;
Line 390: }
Line 391: }
Line 392: List<VmDevice> floppyList =
--
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: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Liron Ar <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Sergey Gotliv <[email protected]>
Gerrit-Reviewer: Tal Nisan <[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
