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

Reply via email to