Maor Lipchuk has posted comments on this change.
Change subject: engine: quota support for multiple storage images
......................................................................
Patch Set 9: (8 inline comments)
Hi Michael, please see my comments inline.
I think using a list for images will gain us more flexibility advantage.
The list should be used also to pass type (raw / preallocated) when creating a
VM from template, and can be used for other commands and operations as well
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java
Line 374: return
QuotaManager.validateMultiStorageQuota(getStoragePool().getQuotaEnforcementType(),
For now this list is only for quota, since I didn't want to get into
integration problems with the GUI if I will use it also for destination storage.
For the long run we should need a list of disks that will contain all the disk
parameters, such as quota id, storage id, tyep...
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromSnapshotCommand.java
Line 313: private Map<Pair<Guid, Guid>, Double> getQuotaConsumeMap() {
I agree, although it will still not cover all the commands.
Unfourtuntily AddVmTemplate or CommonVmPoolWithVmsCommand for example don't
have common inherited class with AddVmFromSnapshotCommand beside commandBase
which I don't want to get it dirty with quota.
Maybe its worth to move this specific method to VmManagementCommandBase, thanks.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommonVmPoolWithVmsCommand.java
Line 82: protected boolean validateQuota() {
Agreed
Line 85: getStoragePool()));
The disks are initialized from the GUI with destination quota id, that is why I
need them from the GUI.
Agreed that the diskInfoList should be synchronized with
imageToDestinationDomainMap here, will be fixed on the next patch.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommand.java
Line 103: // Set default quota id if storage pool enforcement is
disabled.
There should not be use of getParameters() here, only getQuotaId and
setQuotaId. Will be changed in next patch.
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/AddVmPoolWithVmsParameters.java
Line 22: private ArrayList<DiskImage> diskInfoDestinationList;
This is for getting the quota destination for each image when creating a pool
(same like storage domain),
and I use a list since for the long run we should need a list of disks that
will contain all the disk parameters, such as quota id, storage id, type etc..
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/AddVmTemplateParameters.java
Line 28: private HashMap<Guid, DiskImage> diskInfoDestinationMap;
Since on the long run, we should need a list of disks that will contain all
the disk parameters, I think its more flexible this way.
For now quota id and storage id can be kept with pair object, but what will
happen when we will need another attribute to be configured from the GUI.
We will then need to change the type to List or Map. and IMHO maintaining this
will be more difficult then using simply a DiskImage map.
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VmManagementParametersBase.java
Line 23: private ArrayList<DiskImage> diskInfoDestinationList;
This is a parameter class, for VMs basically, there are no child classes
sharing this parameter.
all the other parameter classes (such as template and pool) don't have a
parameter class in common.
--
To view, visit http://gerrit.ovirt.org/3090
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3e4b7e28fb9dd3069e86bd8fdf41255398fd1e58
Gerrit-PatchSet: 9
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Michael Kublin <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
