Re: [Engine-devel] [vdsm] CPU Overcommit Feature

On 12/20/2012 09:43 AM, Dan Kenigsberg wrote: >On Wed, Dec 19, 2012 at 09:53:15AM -0500, Doron Fediuck wrote: >> >>----- Original Message ----- >>>From: "Dan Kenigsberg" <danken(a)redhat.com&gt; >>>To: "Greg Padgett" <gpadgett(a)redhat.com&gt; >>>Cc: "engine-devel" <engine-devel(a)ovirt.org&gt;, vdsm-devel(a)fedorahosted.org >>>Sent: Wednesday, December 19, 2012 3:59:11 PM >>>Subject: Re: [Engine-devel] CPU Overcommit Feature >>> >>>On Mon, Dec 17, 2012 at 09:37:57AM -0500, Greg Padgett wrote: >>>>Hi, >>>> >>>>I've been working on a feature to allow CPU Overcommitment of hosts >>>>in a cluster. This first stage allows the engine to consider host >>>>cpu threads as cores for the purposes of VM resource allocation. >>>> >>>>This wiki page has further details, your comments are welcome! >>>>http://www.ovirt.org/Features/cpu_overcommit >>> >>>I've commented about the vdsm/engine API on >>>http://gerrit.ovirt.org/#/c/10144/ but it is probably better to >>>reiterate it here. >>> >>>The suggested API is tightly coupled with an ugly hack we pushed to >>>vdsm >>>in order not to solve the issue properly on the first strike. >>> >>>If we had not have report_host_threads_as_cores, I think we'd have a >>>simpler API reporting only cpuThreads and cpuCores; with no funny >>>boolean flags. >>> >>>Let us strive to that position as much as we can. >>> >>>How about asking whoever used report_host_threads_as_cores to unset >>>it >>>once they install Engine 3.2 ? I think that these are very few >>>people, >>>that would not mind this very much. >>> >>>If this is impossible, I'd add a cpuCores2, always reporting the true >>>number, to be used by new Engines. We may even report it only on the >>>very few cases of report_host_threads_as_cores being set. >>> >>>Dan. >> >>Hi Dan, >>Thanks for the review. >> >>I agree simply reporting cores and threads would be the right solution. >>However, when you have hyperthreading turned off you get cores=threads. >>This is the same situation you have when hyperthreading turned on, and >>someone used the vdsm configuration of reporting threads as cores. >> >>So the engine won't know the real status of the host. > >This is not surprising, as report_host_threads_as_cores means in blunt >English "lie to Engine about the number of cores". The newly suggested >flag says "don't believe what I said in cpuCores, since I'm lying". Next >thing we'd have is another flag saying that the former flag was a lie, >and cpuCores is actually trustworthy. > >Instead of dancing this dance, I suggest to stop lying. > >report_host_threads_as_cores was a hack to assist a older Engine >versions. Engine users that needed it had to set it out-of-band on their >hosts. Now if they upgrade their Engine, they can -- as easily -- reset >that value. > >If they forget, nothing devastating happens beyond Engine assuming that >hyperthreading is off. > >Please consider this suggestion. I find it the simplest for all involved >parties. the only problem is the new vdsm doesn't know which engine may be using it. if engine would say "getVdsCaps engineVersion=3.2", then vdsm could know engine no longer needs lying to and ignore the flag, re-using same field.