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

[Marcel Apfelbaum]

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 <nik...@linux.vnet.ibm.com>

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