Re: [Qemu-devel] [PATCH 3/5] target-s390: Check for standby memory specification

[Matthew Rosato <mjrosato@linux.vnet.ibm.com>]

On 12/16/2013 04:25 PM, Alexander Graf wrote:
> 
> On 16.12.2013, at 21:51, Matthew Rosato <mjrosato@linux.vnet.ibm.com> wrote:
> 
>> When machine=...,standby-mem={size} has been specified, convert the value
>> to bytes and store it for use.
>>
>> Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
>> ---
>> target-s390x/kvm.c |   16 ++++++++++++++++
>> 1 file changed, 16 insertions(+)
>>
>> diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
>> index 02ac4ba..d4081f4 100644
>> --- a/target-s390x/kvm.c
>> +++ b/target-s390x/kvm.c
>> @@ -97,11 +97,27 @@ static void *legacy_s390_alloc(size_t size);
>>
>> int kvm_arch_init(KVMState *s)
>> {
>> +    int64_t value;
>> +
>>     cap_sync_regs = kvm_check_extension(s, KVM_CAP_SYNC_REGS);
>>     if (!kvm_check_extension(s, KVM_CAP_S390_GMAP)
>>         || !kvm_check_extension(s, KVM_CAP_S390_COW)) {
>>         phys_mem_set_alloc(legacy_s390_alloc);
>>     }
>> +
>> +    value = qemu_opt_get_size(qemu_get_machine_opts(), "standby-mem", -1);
>> +
>> +    if (value < 0) {
>> +        fprintf(stderr, "qemu: invalid standby-mem size:%"PRId64"\n", value);
>> +        exit(1);
>> +    }
>> +
>> +    if (value != (int64_t)(ram_addr_t)value) {
>> +        fprintf(stderr, "qemu: standby size too large\n");
>> +        exit(1);
>> +    }
>> +    standby_mem_size = value * 1024 * 1024;
> 
> I would hope qemu_opt_get_size() returns a value in bytes. Why multiply it here?
> 

It's actually in megabytes, and this is converting to bytes --  Based on
your comment, it sounds like qemu_opt_get_size should always be
returning a byte value.

FWIW, if I adopt Paolo's comments re: patch 1, this behavior goes away
(and the parsing moves out of s390).  Either way, I'll adjust for v2.

> 
> Alex
> 
> 
> 
>