[Ovirt-devel] [PATCH]: Add host memory in kb to the database
slinabery at redhat.com
Mon Aug 11 15:08:17 UTC 2008
On Mon, Aug 11, 2008 at 10:54:19AM -0400, Scott Seago wrote:
> Steve Linabery wrote:
>> On Mon, Aug 11, 2008 at 03:16:29PM +0200, Chris Lalancette wrote:
>>> When inserting memory values into the database, ovirt-identify-node is
>>> sending over values in kilobytes. The database is also in kilobytes.
>>> Don't run a non-sensical "mb_to_kb" conversion on the memory value
>>> before sticking it into the database.
>>> Signed-off-by: Chris Lalancette <clalance at redhat.com>
>>> diff --git a/wui/src/host-browser/host-browser.rb b/wui/src/host-browser/host-browser.rb
>>> index a1bda3d..881b2ae 100755
>>> --- a/wui/src/host-browser/host-browser.rb
>>> +++ b/wui/src/host-browser/host-browser.rb
>>> @@ -219,7 +219,7 @@ class HostBrowser
>>> "hostname" => host_info['HOSTNAME'],
>>> "hypervisor_type" => host_info['HYPERVISOR_TYPE'],
>>> "arch" => host_info['ARCH'],
>>> - "memory_in_mb" => host_info['MEMSIZE'],
>>> + "memory" => host_info['MEMSIZE'],
>>> "is_disabled" => 0,
>>> "hardware_pool" => HardwarePool.get_default_pool,
>>> # Let host-status mark it available when it
>>> @@ -232,7 +232,7 @@ class HostBrowser
>>> host.uuid = host_info['UUID']
>>> host.hostname = host_info['HOSTNAME']
>>> host.arch = host_info['ARCH']
>>> - host.memory_in_mb = host_info['MEMSIZE']
>>> + host.memory = host_info['MEMSIZE']
>>> # delete an existing CPUs and create new ones based on the
>>> Ovirt-devel mailing list
>>> Ovirt-devel at redhat.com
>> I don't think this is necessary. The migration file for CreateHosts (currently wui/src/db/migrate/002_create_hosts.rb) doesn't define memory_in_mb, so this attribute is never being stuck into the database.
> memory_in_mb is a method defined on Host which converts from MB to kb to
> stuff in the db. The idea is when you have a number already in kb, you
> just set host.memory, but if your value is in mb (as it would be if
> you're soliciting user input via the WUI), you set host.memory_in_mb,
> which converts to kb and stores in the db.
>> I'm not sure I understand what effect this:
>> "memory_in_mb" => host_info['MEMSIZE'],
>> was intended to have in host-browser.rb in the first place (creating an instance of Host using a non-existent attribute? Do I just not know ruby well enough to know what's going on here?)
> Well the attribute doesn't exist, but as mentioned above, there's a
> method with this name that sets the proper memory value (assuming what
> you have is in megabytes).
For the record, there is only memory_in_mb= that does what you describe here (but which doesn't work for initializing a new instance).
> On this particular change, I seem to recall a bug a while back where we
> had to change the host browser code to use the _in_mb methods to fix an
> earlier problem. Could it be that the semantics of host_browser has
> changed and that, at one point, we were passing memory in MB?
Seems likely enough.
> In a larger sense (i.e. beyond this particular bugfix, etc.) we've
> talked about how we need a more robust way of dealing with
> storage/memory sizing and conversion/display. As we see here, it's
> currently a bit fragile.
To Chris's original post I say ACK, but not for the original reason stated.
ACK because if we don't set "memory" then the newly created object has no info on the memory at all.
More information about the ovirt-devel