incubator-cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Prachi Damle <Prachi.Da...@citrix.com>
Subject RE: Review Request: CS-15609 Volumes can be created as a part of VM creation when un-allocated space is insufficient on primary storage
Date Wed, 25 Jul 2012 18:17:14 GMT
Hi Mice,

I think option#2 is better because it will avoid unnecessary iterations and return when first
fit satisfying both volume allocations and host linkage is found.

However I think it would be good to do all the size computation in a separate method for readability.

Also following needs to be fixed in your logic:


1)  haveEnoughSpace should be initialized to false for every Volume iteration

2)  If for some volume none of the pool seems to have enough space (due to other prior volumes
allocated), we need not continue - error out there.

Thus add the check if(!haveEnoughSpace) { //error/ } inside the l

3)  storageUsedMap should be initialized for every host iteration, otherwise while trying
the second host 'requestedSpaceSize' might be wrong.


4)  Before iterating over the volumes, I think we should sort the list in descending order
of volume size - so that volume needing the largest chunk of space gets mapped to the pool
first.



This will avoid the situation like this:

volA 4G, volb 6G

SP1 6G, SP2 4G

Allocator could  return volA -> SP1, SP2 and volB -> SP1.
If you choose the pool for VolA first, then VolB might not find a fit. But if we sort by size
and choose pool for VolB first, we might find a destination.


Thanks,
Prachi


From: Mice Xia [mailto:mice_xia@tcloudcomputing.com]
Sent: Wednesday, July 25, 2012 6:43 AM
To: Edison Su
Cc: cloudstack-dev@incubator.apache.org; Prachi Damle; Nitin Mehta
Subject: Re: Review Request: CS-15609 Volumes can be created as a part of VM creation when
un-allocated space is insufficient on primary storage



Hi Edison,

after some further investigation, i found it is non-trivial to put this logic in findSuitablePoolsForVolumes.

method findSuitablePoolsForVolumes is essentially used for listing all storage pools for one
individual volume.(it even gets a parameter 'returnUpTo'). if it has to deal with more then
one volumes, the computation there will be exponential, it is basically a tree recursive search
for all possible volume allocation on storagepools, think about following example.

volA 20G , volB 30G
SP1 40G, sp2 60G

there are three possible allocation:
volA->SP1 volB->SP2
volA->SP2 volB->SP2
volA->SP2 volB->SP1

this seems not very efficient and the return signature may need a change.

back to this multiple volumes capacity check issue, i would like to suggest following two
approaches:
1) put special check logic in findSuitablePoolsForVolumes, but, instead of recursive exploring,
return immediately when the first possible allocation is found.
2) or keep it in findPotentialDeploymentResources, while findSuitablePoolsForVolumes is responsbile
for providing pools for single volume, findPotentialDeploymentResources will check them as
a whole and it will return immediately after find a proper allocation.

please adivce, thanks

Regards
Mice



Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message