Liron Ar has posted comments on this change.
Change subject: core: Add a validation when deactivate ISO domain.
......................................................................
Patch Set 7:
(5 comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommand.java
Line 123: if (getStorageDomain().getStorageDomainType() ==
StorageDomainType.ISO) {
Line 124: List<String> vmNames = getVmsWithAttachedISO();
Line 125: if (!vmNames.isEmpty()) {
Line 126: addCanDoActionMessage(String.format("$VmNames %1$s",
StringUtils.join(vmNames, ",")));
Line 127:
addCanDoActionMessage(VdcBllMessages.ERROR_CANNOT_DEACTIVATE_STORAGE_DOMAIN_WITH_ISO_ATTACHED);
the ENUM name of the message should be changed.
Line 128: return false;
Line 129: }
Line 130: }
Line 131: if (!getParameters().getIsInternal()
Line 124: List<String> vmNames = getVmsWithAttachedISO();
Line 125: if (!vmNames.isEmpty()) {
Line 126: addCanDoActionMessage(String.format("$VmNames %1$s",
StringUtils.join(vmNames, ",")));
Line 127:
addCanDoActionMessage(VdcBllMessages.ERROR_CANNOT_DEACTIVATE_STORAGE_DOMAIN_WITH_ISO_ATTACHED);
Line 128: return false;
merge those two lines to return failCanDo..
Line 129: }
Line 130: }
Line 131: if (!getParameters().getIsInternal()
Line 132: && !getVmDAO()
Line 151: return true;
Line 152: }
Line 153:
Line 154: private List<String> getVmsWithAttachedISO() {
Line 155: List<VmStatic> vms =
getVmStaticDAO().getAllByStoragePoolId(getStorageDomain().getStoragePoolId());
what if all of those vms are down? why would i care to deactivate the iso
domain?
after making this change please also merge the used query with the one at line
132.
Line 156: List<String> vmNames = new ArrayList<>();
Line 157: for (VmStatic vmStatic : vms) {
Line 158: if (isVMHasAttachedISO(vmStatic.getId())) {
Line 159: vmNames.add(vmStatic.getName());
Line 377: }
Line 378: return null;
Line 379: }
Line 380:
Line 381: private static boolean isVMHasAttachedISO(Guid vmId) {
please move this method to VmDeviceUtils
Line 382: List<VmDevice> deviceList =
Line 383: DbFacade.getInstance()
Line 384: .getVmDeviceDao()
Line 385: .getVmDeviceByVmIdAndType(vmId,
Line 382: List<VmDevice> deviceList =
Line 383: DbFacade.getInstance()
Line 384: .getVmDeviceDao()
Line 385: .getVmDeviceByVmIdAndType(vmId,
Line 386: VmDeviceGeneralType.DISK);
I think that a query can be added here, "getVmDeviceWithSpecParam" (we will
pass "path" there) to be used regardless of this.
Line 387: for (VmDevice device : deviceList) {
Line 388: if (device.getDevice() == VmDeviceType.CDROM.toString()
Line 389: || device.getDevice() ==
VmDeviceType.FLOPPY.toString()) {
Line 390: String path = (String)
device.getSpecParams().get("path");
--
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: 7
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Cheryn Tan <cheryn...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Liron Ar <lara...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgot...@redhat.com>
Gerrit-Reviewer: Tal Nisan <tni...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
