search

fog / fog-ovirt

MIT License
11 stars 21 forks source link

Add vm resources max values #73

Closed bagasse closed 5 years ago

bagasse commented 5 years ago

Related to https://projects.theforeman.org/issues/27086, https://github.com/theforeman/foreman/pull/6852#issuecomment-504726272

Add support of some VM resources max values (vcpus, socket, mem).

I've only oVirt 4.3 instances to test so I don't have any values for previous oVirt version, that's why there is only values for 4.1, 4.2 and 4.3 versions.

bagasse commented 5 years ago

@ezr-ondrej thanks for the review, that's fixed :)

bagasse commented 5 years ago

@ezr-ondrej, sorry, totally reworked this to attach it to Datacenter instead of cluster as @shiramax suggested in https://github.com/theforeman/foreman/pull/6852

orrabin commented 5 years ago

@bagasse have you tried removing the max value all together? oVirt should send a message when we pass the max and unless there is a way to get this dynamically from the API (which I understand from the comment linked in the description there isn't) I'm concerned about the values changing each version and how to keep up with the changes.

bagasse commented 5 years ago

@orabin, thanks for suggestion, on the foreman side, as I saw, this is only used on the compute vm attributes form to provide a user feedback before submitting the host form, so yes maybe the solution is to remove this entirely from the foreman form side and that's it. Both solutions are fine to me, one need more code on fog-ovirt side and so more maintenance, the other remove some code on foreman side so decrease maintenance but remove some user feedback on vm creation.

orrabin commented 5 years ago

@bagasse I agree it removes some feedback but in this case it might be worth it to avoid the cost of possibly breaking for each new version on oVirt.

bagasse commented 5 years ago

@orrabin, ok thanks for feedback, closing it