Skip to content
This repository has been archived by the owner on Nov 9, 2020. It is now read-only.

The "--volume-maxcount" option is confusing #676

Closed
shaominchen opened this issue Nov 2, 2016 · 4 comments
Closed

The "--volume-maxcount" option is confusing #676

shaominchen opened this issue Nov 2, 2016 · 4 comments

Comments

@shaominchen
Copy link
Contributor

The "--volume-maxcount" option is confusing:

  1. It's available from "tenant access set" command, but not available from "tenant access add". It's inconsistent across different commands.
  2. The implementation of this option is incorrect: the product of maxcount and maxsize should not exceed the totalsize. But when running "tenant access set", we can assign any random number (even negative) to maxcount
  3. Why do we even need the "maxcount" option? User can always figure out how many volumes they can create by looking at totalsize and maxsize. This option is redundant and introduces unnecessary complexity.

So, I suggest to remove this option.

@lipingxue @msterin

@govint
Copy link
Contributor

govint commented Nov 2, 2016

OTOH the totalSize option can be removed all together. The tenant isn't
using the total size to reserve space nor limit extending a disk (can't do that now anyway?) and if needed it can be derived from the count of disks and the max volume size attrs. Removing max count is also fine as then we define a tenant as the entity with some fixed storage capacity assigned to it (like in Photon) - without checks though.

@msterin
Copy link
Contributor

msterin commented Nov 2, 2016

@shaominchen consistency is a good point, it should be consistent.
As for the need, we heard from a couple of customers (albeit no stats) that it "would be nice to limit the volume count per tenant". The cost is cheap.
The maxcount x maxsize has no direct relations to total size. In thin provisioning, individual limits do no have to be equal total available resource. Users cannot figure out "how many volumes they can create by looking at totalsize and maxsize." due thin provisioning (which is default nowadays) .
So i do not see many reasons to remove it - some value is there + cost is cheap.

About totalSize - I did not catch the reasoning to remove it. The reason to keep it was to put a ceiling to a tenant space reservation.

@shaominchen
Copy link
Contributor Author

shaominchen commented Nov 2, 2016

@msterin Thanks for pointing out the thin provisioning thing - I was not aware of that this is also supported at the volume layer. Looks like it's implicitly or transparently being supported by the back-end.

If that's the case, I agree that we can keep this option since some customers want it. Then we need to fix a couple of things:

  1. Add this option to "tenant access add" command
  2. Check the validity of the parameter value. For now we can even set a negative value
  3. Add one more column for this option in the "tenant access ls" output - currently there's no column for this option:

[root@localhost:/usr/lib/vmware/vmdkops/bin] vmdkops_admin tenant access ls --name MyTenant
Datastore Create_volume Delete_volume Mount_volume Max_volume_size Total_size


datastore1 1 1 1 512.00MB 1.00GB

@pdhamdhere
Copy link
Contributor

"--volume-maxcount" removed in PR #729

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

No branches or pull requests

4 participants