Maor Lipchuk has posted comments on this change. Change subject: core: Add alias and description for disk meta data ......................................................................
Patch Set 14: (1 comment) http://gerrit.ovirt.org/#/c/34163/14/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java: Line 356: } Line 357: Line 358: private void performDiskUpdate(final boolean unlockImage) { Line 359: if (shouldPerformMetadataUpdate()) { Line 360: updateMetaDataDescription((DiskImage) getNewDisk()); > continuing the discussion for the previous patchset. imo we should lock the The reason we lock the image in the DB is simply to make it persistent so if the engine will crash we will still have an indication that there is an operation in VDSM which is running. this is not the case here, there is no obligation to mark the image as locked in the DB, IMO the memory lock should be enough. Regarding the UI, that is why I was against doing this operation on the update process, I also warned about this regression before, but that is what was decided since we also do it today when we extend the file size, so this is not considered a regression. Line 361: } Line 362: final Disk disk = getDiskDao().get(getParameters().getDiskId()); Line 363: applyUserChanges(disk); Line 364: -- To view, visit http://gerrit.ovirt.org/34163 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie2642ae7016579ead699509426e01ac2010bd374 Gerrit-PatchSet: 14 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Liron Aravot <[email protected]> Gerrit-Reviewer: Maor Lipchuk <[email protected]> Gerrit-Reviewer: Tal Nisan <[email protected]> Gerrit-Reviewer: [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
