Maor Lipchuk has posted comments on this change.
Change subject: core: Add a validation when deactivate ISO domain.
......................................................................
Patch Set 12:
(3 comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommand.java
Line 116: if (!filterDomainsByStatus(domains,
StorageDomainStatus.Locked).isEmpty()) {
Line 117: return
failCanDoAction(VdcBllMessages.ERROR_CANNOT_DEACTIVATE_MASTER_WITH_LOCKED_DOMAINS);
Line 118: }
Line 119: }
Line 120: if (getStorageDomain().getStorageDomainType() ==
StorageDomainType.ISO) {
right! will fix
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, ",")));
Line 151: if (getVmDynamicDAO().get(vmStatic.getId()).getStatus()
!= VMStatus.Down
Line 152: &&
VmDeviceUtils.isVMHasAttachedISO(vmStatic.getId())) {
Line 153: vmNames.add(vmStatic.getName());
Line 154: }
Line 155: }
fixing VmDao.getAllActiveForStorageDomain to work with ISO domain will only
make the method to be more complex and hard to maintain.
If we will load the vms with attached ISO we will still need to add a new
stored procedure, and we will still need to go over this in Java.
I think the fix should be or in the DB or in Java
The question is what would be better, to implement a specific new stored
procedure in the DB that will only be used for that specific purpose or to
implement it in Java and lose some of performance.
IMHO since deactivate ISO Storage Domain, should happen very rarely, it will be
better to implement it in Java and save the maintenance of a new stored
procedure that will be added only for this specific case.
Line 156: return vmNames;
Line 157: }
Line 158:
Line 159: /**
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/utils/VmDeviceUtils.java
Line 110: VmDeviceGeneralType.DISK);
Line 111: for (VmDevice device : deviceList) {
Line 112: if
(device.getDevice().equals(VmDeviceType.CDROM.toString())
Line 113: ||
device.getDevice().equals(VmDeviceType.FLOPPY.toString())) {
Line 114: String path = (String)
device.getSpecParams().get("path");
CDROM and FLOPPY can not be unplugged
Line 115: if (!StringUtils.isEmpty(path)) {
Line 116: return true;
Line 117: }
Line 118: }
--
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: 12
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
