Re: [PATCH kvmtool 06/16] builtin-run.c: Always use ram_size in bytes

[Suzuki Kuruppassery Poulose <suzuki.poulose@arm.com>]

On 23/09/2019 14:35, Alexandru Elisei wrote:
> The user can specify the virtual machine memory size, in MB, which is saved
> in cfg->ram_size. kvmtool validates it against the host memory size,
> converted from bytes to MB. ram_size is aftwerwards converted to bytes, and
> this is how it is used throughout the rest of the program.
> 
> Let's avoid any confusion about the unit of measurement and always use
> cfg->ram_size in bytes.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>   builtin-run.c            | 19 ++++++++++---------
>   include/kvm/kvm-config.h |  2 +-
>   include/kvm/kvm.h        |  2 +-
>   3 files changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/builtin-run.c b/builtin-run.c
> index cff44047bb1c..4e0c52b3e027 100644
> --- a/builtin-run.c
> +++ b/builtin-run.c
> @@ -262,7 +262,7 @@ static u64 host_ram_size(void)
>   		return 0;
>   	}
>   
> -	return (nr_pages * page_size) >> MB_SHIFT;
> +	return nr_pages * page_size;
>   }
>   
>   /*
> @@ -276,11 +276,11 @@ static u64 get_ram_size(int nr_cpus)
>   	u64 available;
>   	u64 ram_size;
>   
> -	ram_size	= 64 * (nr_cpus + 3);
> +	ram_size	= (64 * (nr_cpus + 3)) << MB_SHIFT;
>   
>   	available	= host_ram_size() * RAM_SIZE_RATIO;

The host_ram_size() gives you size in MB isn't it ? You need to
fix that to make sure we aren't comparing "ram_size" in bytes
with "available" in MB below. So the best option is to
talk bytes everywhere.

Otherwise looks fine.

Suzuki