Re: [Qemu-devel] [Qemu-ppc] [PATCH v2] Introduce machine specific default memory size

[Marcel Apfelbaum <marcel.apfelbaum@gmail.com>]

On 03/04/2015 02:21 PM, Alexander Graf wrote:
>
>
> On 04.03.15 12:18, Nikunj A Dadhania wrote:
>> QEMU default memory of 128MB is not enough to boot sPAPR
>> guest. Introduce a member in the machine class to override the default
>> memory size enforced by QEMU.
>>
>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>> ---
>>   hw/ppc/spapr.c         | 1 +
>>   include/hw/boards.h    | 1 +
>>   include/hw/ppc/spapr.h | 3 +++
>>   vl.c                   | 9 +++++++++
>>   4 files changed, 14 insertions(+)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 23cde20..f6b1137 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -1738,6 +1738,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
>>       mc->max_cpus = MAX_CPUS;
>>       mc->no_parallel = 1;
>>       mc->default_boot_order = NULL;
>> +    mc->default_ram_size = SPAPR_DEFAULT_RAM_SIZE;
>>       mc->kvm_type = spapr_kvm_type;
>>       mc->has_dynamic_sysbus = true;
>>
>> diff --git a/include/hw/boards.h b/include/hw/boards.h
>> index 3ddc449..b2b4698 100644
>> --- a/include/hw/boards.h
>> +++ b/include/hw/boards.h
>> @@ -108,6 +108,7 @@ struct MachineClass {
>>       const char *default_display;
>>       GlobalProperty *compat_props;
>>       const char *hw_version;
>> +    ram_addr_t default_ram_size;
>>
>>       HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
>>                                              DeviceState *dev);
>> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
>> index 716bff4..d401dd0 100644
>> --- a/include/hw/ppc/spapr.h
>> +++ b/include/hw/ppc/spapr.h
>> @@ -444,6 +444,9 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr rtas_addr,
>>   #define SPAPR_VIO_BASE_LIOBN    0x00000000
>>   #define SPAPR_PCI_BASE_LIOBN    0x80000000
>>
>> +/* Default to 1GB guest ram_size */
>> +#define SPAPR_DEFAULT_RAM_SIZE  (1ULL << 30)
>> +
>>   #define RTAS_ERROR_LOG_MAX      2048
>>
>>   typedef struct sPAPRTCETable sPAPRTCETable;
>> diff --git a/vl.c b/vl.c
>> index 801d487..447b993 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -2649,6 +2649,7 @@ static void set_memory_options(uint64_t *ram_slots, ram_addr_t *maxram_size)
>>       const ram_addr_t default_ram_size = (ram_addr_t)DEFAULT_RAM_SIZE *
>>                                           1024 * 1024;
>>       QemuOpts *opts = qemu_find_opts_singleton("memory");
>> +    MachineClass *machine_class;
>>
>>       sz = 0;
>>       mem_str = qemu_opt_get(opts, "size");
>> @@ -2684,6 +2685,14 @@ static void set_memory_options(uint64_t *ram_slots, ram_addr_t *maxram_size)
>>           exit(EXIT_FAILURE);
>>       }
>>
>> +    machine_class = find_default_machine();
>> +    if (machine_class->default_ram_size && ram_size < machine_class->default_ram_size) {
>> +        fprintf(stderr, "qemu: %s guest ram size defaulting to %ld MB\n",
>> +                machine_class->name,
>> +                machine_class->default_ram_size / (1024 * 1024));
>> +        ram_size = machine_class->default_ram_size;
>
> Can't the check happen at the same spot that the current 128MB default
> gets set?
>
> Also, why so complicated? Just have a default value of 128MB in the
> default machine class and just override it from the sPAPR one. Then
> unconditionally set ram_size = mc->default_ram_size in the code that
> today sets ram_size = 128MB.
+1

That's what I said :), but much more concise.
Thanks,
Marcel

>
>
> Alex
>
>> +    }
>> +
>>       /* store value for the future use */
>>       qemu_opt_set_number(opts, "size", ram_size, &error_abort);
>>       *maxram_size = ram_size;
>>
>