Moti Asayag has posted comments on this change. Change subject: core: ensure unique unit for VirtIO_SCSI and SPAR_VSCSI disks ......................................................................
Patch Set 4: Code-Review+1 (6 comments) +1 for this approach. https://gerrit.ovirt.org/#/c/39471/4/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AbstractDiskVmCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AbstractDiskVmCommand.java: Line 352: } Line 353: } else { Line 354: try { Line 355: lockVmDiskHotPlugWithWait(); Line 356: VM vm = DbFacade.getInstance().getVmDao().get(getParameters().getVmId()); please use injection for VmDao Line 357: Map<DiskInterface, Integer> controllerIndexMap = Line 358: ArchStrategyFactory.getStrategy(vm.getClusterArch()).run(new GetControllerIndices()).returnValue(); Line 359: Line 360: int virtioScsiIndex = controllerIndexMap.get(DiskInterface.VirtIO_SCSI); Line 390: if (vmDeviceUnitMap.containsValue(unit)) { Line 391: addressMap = VmInfoBuilder.createAddressForScsiDisk(controllerIndex, availableUnit); Line 392: } Line 393: } Line 394: else { please merge with former line. Line 395: addressMap = VmInfoBuilder.createAddressForScsiDisk(controllerIndex, availableUnit); Line 396: } Line 397: Line 398: // Updating device's address immediately (instead of waiting to VmsMonitoring) Line 404: Line 405: protected void updateVmDeviceAddress(final String address, final VmDevice vmDevice) { Line 406: // could be overridden in sub-classes commands using 'getDiskAddressMap' method Line 407: vmDevice.setAddress(address); Line 408: DbFacade.getInstance().getVmDeviceDao().update(vmDevice); please use injection for VmDeviceDao Line 409: } Line 410: Line 411: protected void lockVmDiskHotPlugWithWait() { Line 412: vmDiskHotPlugEngineLock = new EngineLock(); https://gerrit.ovirt.org/#/c/39471/4/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AttachDiskToVmCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AttachDiskToVmCommand.java: Line 26: import org.ovirt.engine.core.common.businessentities.VmDeviceGeneralType; Line 27: import org.ovirt.engine.core.common.businessentities.VmDeviceId; Line 28: import org.ovirt.engine.core.common.businessentities.storage.Disk; Line 29: import org.ovirt.engine.core.common.businessentities.storage.DiskImage; Line 30: import org.ovirt.engine.core.common.businessentities.storage.DiskStorageType; this file can be pulled off this patch. Line 31: import org.ovirt.engine.core.common.businessentities.storage.ImageStatus; Line 32: import org.ovirt.engine.core.common.errors.VdcBLLException; Line 33: import org.ovirt.engine.core.common.errors.VdcBllErrors; Line 34: import org.ovirt.engine.core.common.errors.VdcBllMessages; https://gerrit.ovirt.org/#/c/39471/4/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HotPlugDiskToVmCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HotPlugDiskToVmCommand.java: Line 157: TransactionSupport.executeInNewTransaction(new TransactionMethod<Void>() { Line 158: @Override Line 159: public Void runInTransaction() { Line 160: vmDevice.setAddress(address); Line 161: DbFacade.getInstance().getVmDeviceDao().update(vmDevice); please use injection for VmDeviceDao Line 162: return null; Line 163: } Line 164: }); Line 165: } https://gerrit.ovirt.org/#/c/39471/4/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/HotPlugDiskVDSParameters.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/HotPlugDiskVDSParameters.java: Line 49: public void setVm(VM vm) { Line 50: this.vm = vm; Line 51: } Line 52: Line 53: public Map<String, String> getAddressMap() { would you like adding it to the appendAttributes(sb) so it will be printed to log when command is executed ? Line 54: return addressMap; Line 55: } Line 56: Line 57: public void setAddressMap(Map<String, String> addressMap) { -- To view, visit https://gerrit.ovirt.org/39471 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5e621618e8891fca52b48bff437cc4c2a1f695db Gerrit-PatchSet: 4 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Daniel Erez <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Arik Hadas <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot <[email protected]> Gerrit-Reviewer: Moti Asayag <[email protected]> Gerrit-Reviewer: Omer Frenkel <[email protected]> Gerrit-Reviewer: Roy Golan <[email protected]> Gerrit-Reviewer: Vitor de Lima <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
