Maor Lipchuk has posted comments on this change. Change subject: core: introduce RemoveDiskSnapshotsCommand ......................................................................
Patch Set 7: (2 comments) http://gerrit.ovirt.org/#/c/26327/7/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskSnapshotsCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskSnapshotsCommand.java: Line 281: Line 282: long sizeRequested = 0L; Line 283: for (DiskImage image : getImages()) { Line 284: sizeRequested += (long) image.getActualSize(); Line 285: } as discussed I think that the best way should be: min(sum(actual_size), max(virtual_size)) If doing so, please add a comment of explenation Line 286: Line 287: if (!validate(validator.isDomainHasSpaceForRequest(sizeRequested, false))) { Line 288: return false; Line 289: } http://gerrit.ovirt.org/#/c/26327/7/backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties File backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties: Line 54: USER_REMOVE_SNAPSHOT_FINISHED_FAILURE=Failed to delete snapshot '${SnapshotName}' for VM '${VmName}'. Line 55: USER_REMOVE_DISK_SNAPSHOT=Disk '${DiskAlias}' from Snapshot(s) '${Snapshots}' of VM '${VmName}' deletion was initiated by ${UserName}. Line 56: USER_FAILED_REMOVE_DISK_SNAPSHOT=Failed to delete Disk '${DiskAlias}' from Snapshot(s) ${Snapshots} of VM ${VmName} (User: ${UserName}). Line 57: USER_REMOVE_DISK_SNAPSHOT_FINISHED_SUCCESS=Disk '${DiskAlias}' from Snapshot(s) '${Snapshots}' of VM '${VmName}' deletion has been completed. Line 58: USER_REMOVE_DISK_SNAPSHOT_FINISHED_FAILURE=Failed to complete deletion of Disk '${DiskAlias}' from snapshot(s) '${Snapshots}' of VM '${VmName}'. Don't you want to also indicate the ${UserName} at USER_REMOVE_DISK_SNAPSHOT_FINISHED_SUCCESS and USER_REMOVE_DISK_SNAPSHOT_FINISHED_FAILURE, so it can be searched later Line 59: USER_DETACH_USER_FROM_POOL=User ${AdUserName} was detached from VM Pool ${VmPoolName} by ${UserName}. Line 60: USER_DETACH_USER_FROM_POOL_FAILED=Failed to detach User ${AdUserName} from VM Pool ${VmPoolName} (User: ${UserName}). Line 61: USER_DETACH_USER_FROM_VM=User ${AdUserName} was detached from VM ${VmName} by ${UserName}. Line 62: USER_FAILED_DETACH_USER_FROM_VM=Failed to detach User ${AdUserName} from VM ${VmName} (User: ${UserName}). -- To view, visit http://gerrit.ovirt.org/26327 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia714a4390d1d9b672005be30f58b7fa98b9a31cd Gerrit-PatchSet: 7 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Liron Ar <lara...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Tal Nisan <tni...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
