[Ovirt-devel] [PATCH]: Add host memory in kb to the database

[Scott Seago]

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']
>>          end
>>  
>>          # delete an existing CPUs and create new ones based on the data
>>
>> _______________________________________________
>> Ovirt-devel mailing list
>> Ovirt-devel at redhat.com
>> https://www.redhat.com/mailman/listinfo/ovirt-devel
>>     
>
> 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).

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?

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.

Scott
> Thanks,
> Steve
>
> _______________________________________________
> Ovirt-devel mailing list
> Ovirt-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/ovirt-devel
>