Allon Mureinik has posted comments on this change.
Change subject: core: Add a validation when deactivate ISO domain.
......................................................................
Patch Set 7: Code-Review-1
(2 comments)
-1 on the usage of == instead of equals, which is potentially buggy,
@Cheryn - can you take a look at the new message at
backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
please?
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommand.java
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<>();
Nit: A LinkedList would perform better here
Line 157: for (VmStatic vmStatic : vms) {
Line 158: if (isVMHasAttachedISO(vmStatic.getId())) {
Line 159: vmNames.add(vmStatic.getName());
Line 160: }
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()) {
You are comparing strings - there is no guarantee that "==" would work,
especially if your values are coming from the database.
Use equals() instead.
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
