Liron Ar has posted comments on this change. Change subject: core: add the image block alignment scan ......................................................................
Patch Set 6: (5 inline comments) .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ScanDiskAlignmentCommand.java Line 65: Line 66: @Override Line 67: protected Map<String, Pair<String, String>> getSharedLocks() { Line 68: if (getDisk() != null && !isImageExclusiveLockNeeded()) { Line 69: return Collections.singletonMap(getDisk().getId().toString(), LockMessagesMatchUtil.DISK); we need to add a message that will be displayed in case that there will be attempt to acquire exclusive lock Line 70: } Line 71: return null; Line 72: } Line 73: Line 93: } Line 94: Line 95: if (getDiskType() == DiskStorageType.IMAGE) { Line 96: if (((DiskImage) getDisk()).getImageStatus() != ImageStatus.OK) { Line 97: return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_DISKS_LOCKED); the image can also be illegal for example, can you change the message? Line 98: } Line 99: } Line 100: Line 101: if (getVm() == null) { Line 145: Line 146: parameters = lunParameters; Line 147: } else { Line 148: throw new VdcBLLException(VdcBllErrors.ENGINE, "Unknown DiskStorageType: " + Line 149: getDiskType().toString() + " Disk id: " + getDisk().getId().toString()); this is unneeded - please remove it, otherwise we need to put it all over the code Line 150: } Line 151: Line 152: Boolean isDiskAligned = (Boolean) runVdsCommand( Line 153: VDSCommandType.ScanDiskAlignment, parameters).getReturnValue(); Line 157: Line 158: getBaseDiskDao().update(getDisk()); Line 159: setSucceeded(true); Line 160: Line 161: releaseExclusiveDiskLocks(); all of those db operations should be done in the same transaction - otherwise in case of engine crash for example, the image will remain locked. Line 162: } Line 163: Line 164: @Override Line 165: public List<PermissionSubject> getPermissionCheckSubjects() { Line 216: } Line 217: Line 218: protected Guid getVdsIdInPool() { Line 219: if (vdsInPool == null && getStoragePoolId() != null) { Line 220: for (VDS vds : getVdsDAO().getAllForStoragePoolAndStatus( perhaps remove the for loop from here. Line 221: getStoragePoolId(), VDSStatus.Up)) { Line 222: vdsInPool = vds.getId(); Line 223: break; // At the moment we select the first VDS in Up status Line 224: } -- To view, visit http://gerrit.ovirt.org/11946 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4858b7bbfa453230fcafecfbc5358c715d5d825b Gerrit-PatchSet: 6 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli <fsimo...@redhat.com> Gerrit-Reviewer: Alissa Bonas <abo...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com> Gerrit-Reviewer: Liron Ar <lara...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches