Maor Lipchuk has posted comments on this change. Change subject: core: Use storage validator when validating move or copy. ......................................................................
Patch Set 1: (2 inline comments) .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommand.java Line 151: return false; Line 152: } Line 153: Line 154: getImage().getSnapshots().addAll(getAllImageSnapshots()); Line 155: if (!doesStorageDomainHaveSpaceForRequest(Math.round(getImage().getActualDiskWithSnapshotsSize()))) { I think the problem is much more basic, this specific storage space check is problematic. 1. We do not take care of races. For example you can do parallel commands like adding disks, creating a VM pool and a template, and each one of this commands, will pass the CDA but still consume the storage much more then the threshold. 2. We now count block size for sparse as 1GB, although engine can not be sure what will be the size of a block since VDSM does not expose you that value, and probably can not tell the engine how much that will be before starting to allocate. We should need to decide how we can improve this mechanism with VDSM help, and probably send a patch that can be verified properly. Line 156: addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_DISK_SPACE_LOW); Line 157: return false; Line 158: } Line 159: return true; Line 153: Line 154: getImage().getSnapshots().addAll(getAllImageSnapshots()); Line 155: if (!doesStorageDomainHaveSpaceForRequest(Math.round(getImage().getActualDiskWithSnapshotsSize()))) { Line 156: addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_DISK_SPACE_LOW); Line 157: return false; actually since using the validator, it only need to return false now. Line 158: } Line 159: return true; Line 160: } Line 161: -- To view, visit http://gerrit.ovirt.org/11513 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I01980de86ed445d397f12ff9c471974d1cb5c44d Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@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