Sergey Gotliv has posted comments on this change. Change subject: engine: Warn user when a boot disk is configured as read-only ......................................................................
Patch Set 1: (2 comments) http://gerrit.ovirt.org/#/c/24883/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java: Line 344: Line 345: @Override Line 346: protected void executeVmCommand() { Line 347: if (getParameters().getDiskInfo().isBoot() && getParameters().getDiskInfo().getReadOnly()) { Line 348: AuditLogDirector.log(this, AuditLogType.BOOT_DISK_IS_READ_ONLY); > there's an issue here, we might fail to create this disk while still have t I am warning a user about the possible error. I can consider another place to put this code but 2 different places for images and luns is probably too much. Anyway I think this is good enough for such a corner case, but I wouldn't mind if someone will move this code to the better place. Line 349: } Line 350: Line 351: getParameters().getDiskInfo().setId(Guid.newGuid()); Line 352: getParameters().setEntityInfo(new EntityInfo(VdcObjectType.Disk, getParameters().getDiskInfo().getId())); http://gerrit.ovirt.org/#/c/24883/1/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 112: @Override Line 113: protected void executeVmCommand() { Line 114: ImagesHandler.setDiskAlias(getParameters().getDiskInfo(), getVm()); Line 115: Line 116: if (getNewDisk().isBoot() && getNewDisk().getReadOnly()) { > this will appear now on every update to the disk, even if we only changed i I tend to agree with you. My original position was warn him as much as possible, because disk can be reformatted and new filesystem can be installed on it... Line 117: AuditLogDirector.log(this, AuditLogType.BOOT_DISK_IS_READ_ONLY); Line 118: } Line 119: Line 120: if (shouldResizeDiskImage()) { -- To view, visit http://gerrit.ovirt.org/24883 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I89dfc4492352b650eeafd89723e3d50de19bd5eb Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Sergey Gotliv <[email protected]> Gerrit-Reviewer: A Burden <[email protected]> Gerrit-Reviewer: Allon Mureinik <[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: [email protected] Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
