Liron Ar 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) {
there's a bug here, it should be checked that the execution isn't internal
!getParameters().getIsInternal() otherwise the domain fail process won't work.
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: }
The queries here will load huge parts of the db, we should discuss whether we
want this logic here or just fix VmDao.getAllActiveForStorageDomain or
something to work also for ISO domains as this stored procedure doesn't work
properly for iso domains.
if we do want to keep this logic out of the db, it might be better to maybe
load only the vms of the pool with plugged iso and test them, it will reduce
the overload there by a lot.
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");
what if the "iso device" is unplugged? we should get false then.
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
