From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55045) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YT8OE-0005CT-Jp for qemu-devel@nongnu.org; Wed, 04 Mar 2015 07:27:06 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YT8OD-00049t-Jo for qemu-devel@nongnu.org; Wed, 04 Mar 2015 07:27:02 -0500 Message-ID: <54F6FA0B.9090200@gmail.com> Date: Wed, 04 Mar 2015 14:26:51 +0200 From: Marcel Apfelbaum MIME-Version: 1.0 References: <1425467885-26712-1-git-send-email-nikunj@linux.vnet.ibm.com> In-Reply-To: <1425467885-26712-1-git-send-email-nikunj@linux.vnet.ibm.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2] Introduce machine specific default memory size Reply-To: marcel@redhat.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Nikunj A Dadhania , qemu-devel@nongnu.org Cc: qemu-ppc@nongnu.org, armbru@redhat.com, thuth@linux.vnet.ibm.com On 03/04/2015 01:18 PM, 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 Hi, Thank you for contributing to QEMU! > --- > 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; You add a field to MachineClass that represents default_ram_size. That's fair because different machine types may have a different "native" default. But in this case you should set DEFAULT_RAM_SIZE as default for all machine types for consistency. (as suggested also by others) > > 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(); I am not sure this is want you want to do here. I think you want to use the *current* machine class and not the default one. that can be passed as parameter to set_memory_options. > + if (machine_class->default_ram_size && ram_size < machine_class->default_ram_size) { and here you can drop the un-natural machine_class->default_ram_size check > + 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; > Thanks, Marcel