Maor Lipchuk has posted comments on this change.
Change subject: core: Add a validation when deactivate ISO domain.
......................................................................
Patch Set 7:
(8 comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommand.java
Line 120: return false;
Line 121: }
Line 122: }
Line 123: if (getStorageDomain().getStorageDomainType() ==
StorageDomainType.ISO) {
Line 124: List<String> vmNames = getVmsWithAttachedISO();
I don't like to assign at the middle of a condition, it will just make it more
clumsy
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;
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);
What do you suggest?
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;
done
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());
so... they will still be configured with an ISO file, I think we should still
block this since the user can later on activate this VM, but I don't mind to
hear more opinions about it
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 152: }
Line 153:
Line 154: private List<String> getVmsWithAttachedISO() {
Line 155: List<VmStatic> vms =
getVmStaticDAO().getAllByStoragePoolId(getStorageDomain().getStoragePoolId());
Line 156: List<String> vmNames = new ArrayList<>();
done
Line 157: for (VmStatic vmStatic : vms) {
Line 158: if (isVMHasAttachedISO(vmStatic.getId())) {
Line 159: vmNames.add(vmStatic.getName());
Line 160: }
Line 377: }
Line 378: return null;
Line 379: }
Line 380:
Line 381: private static boolean isVMHasAttachedISO(Guid vmId) {
done.
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);
This is a unique scenario I don't see the need to add a new stored procedure
for that one
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");
Line 385: .getVmDeviceByVmIdAndType(vmId,
Line 386: VmDeviceGeneralType.DISK);
Line 387: for (VmDevice device : deviceList) {
Line 388: if (device.getDevice() == VmDeviceType.CDROM.toString()
Line 389: || device.getDevice() ==
VmDeviceType.FLOPPY.toString()) {
done
Line 390: String path = (String)
device.getSpecParams().get("path");
Line 391: if (!StringUtils.isEmpty(path)) {
Line 392: return true;
Line 393: }
--
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 <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Cheryn Tan <[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
