On Wed, 04 Mar 2015 14:32:44 +0200 Marcel Apfelbaum <marcel.apfelb...@gmail.com> wrote:
> On 03/04/2015 02:21 PM, Igor Mammedov wrote: > > On Wed, 4 Mar 2015 16:48:05 +0530 > > Nikunj A Dadhania <nik...@linux.vnet.ibm.com> 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. > >> > > now we have DEFAULT_RAM_SIZE in vl.c and default_ram_size in Machine class. > > it is confusing. > Agree, but moving the define into hw/core/machine.c and setting it as > base class default would make this clean. > > > > Could you make current ram_size in vl.c a property of Machine > > with default DEFAULT_RAM_SIZE moved there for generic Machine class > > and do what you are doing here in property setter. Then it will work > > for all machines the same way. > Once is a property with a setter it can be set by command line! > I don't think this is the right think to do. Why is it not right? We could some day abandon -m and just use -machine property parsing. Another purpose of it is to move machine related code i.e. parsing/ setting/getting ram_size, slots, maxmem to machine class and drop adhoc parsing and globals in vl.c. > > Thanks, > Marcel > > > > > I'd actually suggest to take set_memory_options() and make > > ram_size, slots, maxmem a properties of Machine and remap > > -m parsing to normal machine option parsing. Getting rid of > > global ram_size along the way. > > > > > >> Signed-off-by: Nikunj A Dadhania <nik...@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; > >> + } > >> + > >> /* store value for the future use */ > >> qemu_opt_set_number(opts, "size", ram_size, &error_abort); > >> *maxram_size = ram_size; > > > > >