On Thu, 5 Mar 2015 14:36:10 +0530
Nikunj A Dadhania <nik...@linux.vnet.ibm.com> wrote:
> Machines types can have different requirement for default ram
> size. Introduce a member in the machine class and set the current
> default_ram_size to 128MB.
>
> For QEMUMachine types override the value during the registration of
> the machine and for MachineClass introduce the generic class init
> setting the default_ram_size.
>
> In case the user passes memory that is lesser that the default ram
> size, upscale the value to the machine's default ram size with a
> warning.
>
> Signed-off-by: Nikunj A Dadhania <nik...@linux.vnet.ibm.com>
> ---
> hw/core/machine.c | 8 ++++++++
> include/hw/boards.h | 4 ++++
> vl.c | 29 +++++++++++++++++------------
> 3 files changed, 29 insertions(+), 12 deletions(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index fbd91be..29c48de 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -403,11 +403,19 @@ bool machine_usb(MachineState *machine)
> return machine->usb;
> }
>
> +static void machine_class_init(ObjectClass *oc, void *data)
> +{
> + MachineClass *mc = MACHINE_CLASS(oc);
> +
> + mc->default_ram_size = MACHINE_DEFAULT_RAM_SIZE;
> +}
> +
> static const TypeInfo machine_info = {
> .name = TYPE_MACHINE,
> .parent = TYPE_OBJECT,
> .abstract = true,
> .class_size = sizeof(MachineClass),
> + .class_init = machine_class_init,
> .instance_size = sizeof(MachineState),
> .instance_init = machine_initfn,
> .instance_finalize = machine_finalize,
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 3ddc449..3fca4e0 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -62,6 +62,9 @@ int qemu_register_machine(QEMUMachine *m);
> #define MACHINE_CLASS(klass) \
> OBJECT_CLASS_CHECK(MachineClass, (klass), TYPE_MACHINE)
>
> +/* Default 128 MB as guest ram size */
> +#define MACHINE_DEFAULT_RAM_SIZE (1UL << 27)
no need for it to be global, bury it inside hw/core/machine.c
> +
> MachineClass *find_default_machine(void);
> extern MachineState *current_machine;
>
> @@ -108,6 +111,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/vl.c b/vl.c
> index 801d487..7af1c0b 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -120,8 +120,6 @@ int main(int argc, char **argv)
> #include "qom/object_interfaces.h"
> #include "qapi-event.h"
>
> -#define DEFAULT_RAM_SIZE 128
> -
> #define MAX_VIRTIO_CONSOLES 1
> #define MAX_SCLP_CONSOLES 1
>
> @@ -1333,6 +1331,7 @@ static void machine_class_init(ObjectClass *oc, void
> *data)
Now it's confusing, we have 2 machine_class_init() one for TYPE_MACHINE
in hw/core/machine.c and another one here for subclasses.
Maybe rename this one to qemu_machine_class_init() with comment that it's
transitional class registration used for converting from legacy QEMUMachine
to MachineClass.
> mc->is_default = qm->is_default;
> mc->default_machine_opts = qm->default_machine_opts;
> mc->default_boot_order = qm->default_boot_order;
> + mc->default_ram_size = MACHINE_DEFAULT_RAM_SIZE;
this looks wrong, default_ram_size is initialized when TYPE_MACHINE
class is initialized. No need to override the same default in immediate subclass
> mc->default_display = qm->default_display;
> mc->compat_props = qm->compat_props;
> mc->hw_version = qm->hw_version;
> @@ -2641,13 +2640,13 @@ out:
> return 0;
> }
>
> -static void set_memory_options(uint64_t *ram_slots, ram_addr_t *maxram_size)
> +static void set_memory_options(uint64_t *ram_slots, ram_addr_t *maxram_size,
> + MachineClass *mc)
> {
> uint64_t sz;
> const char *mem_str;
> const char *maxmem_str, *slots_str;
> - const ram_addr_t default_ram_size = (ram_addr_t)DEFAULT_RAM_SIZE *
> - 1024 * 1024;
> + const ram_addr_t default_ram_size = mc->default_ram_size;
> QemuOpts *opts = qemu_find_opts_singleton("memory");
>
> sz = 0;
> @@ -2684,6 +2683,12 @@ static void set_memory_options(uint64_t *ram_slots,
> ram_addr_t *maxram_size)
> exit(EXIT_FAILURE);
> }
>
> + if (ram_size < default_ram_size) {
> + fprintf(stderr, "WARNING: qemu: %s guest ram size defaulting to %ld
> MB\n",
> + mc->name, default_ram_size / (1024 * 1024));
> + ram_size = default_ram_size;
> + }
In previous review someone explicitly asked not to override lower ram_size
if it was requested by user on command line.
> +
> /* store value for the future use */
> qemu_opt_set_number(opts, "size", ram_size, &error_abort);
> *maxram_size = ram_size;
> @@ -3763,7 +3768,13 @@ int main(int argc, char **argv, char **envp)
> machine_class = machine_parse(optarg);
> }
>
> - set_memory_options(&ram_slots, &maxram_size);
> + if (machine_class == NULL) {
> + fprintf(stderr, "No machine specified, and there is no default.\n"
> + "Use -machine help to list supported machines!\n");
> + exit(1);
> + }
> +
> + set_memory_options(&ram_slots, &maxram_size, machine_class);
>
> loc_set_none();
>
> @@ -3792,12 +3803,6 @@ int main(int argc, char **argv, char **envp)
> }
> #endif
>
> - if (machine_class == NULL) {
> - fprintf(stderr, "No machine specified, and there is no default.\n"
> - "Use -machine help to list supported machines!\n");
> - exit(1);
> - }
> -
> current_machine = MACHINE(object_new(object_class_get_name(
> OBJECT_CLASS(machine_class))));
> if (machine_help_func(qemu_get_machine_opts(), current_machine)) {
