[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/4] machine: convert ram_size, maxram_size, ram
|
From: |
Igor Mammedov |
|
Subject: |
Re: [Qemu-devel] [PATCH 3/4] machine: convert ram_size, maxram_size, ram_slots to properties |
|
Date: |
Wed, 25 Jun 2014 15:08:03 +0200 |
On Wed, 25 Jun 2014 14:54:47 +0300
"Michael S. Tsirkin" <address@hidden> wrote:
> On Wed, Jun 25, 2014 at 01:42:22PM +0200, Igor Mammedov wrote:
> > ... short term it will help writing unit tests accessing
> > these values via QMP. And down the road it will allow to drop
> > global variable ram_size.
> >
> > Signed-off-by: Igor Mammedov <address@hidden>
> > ---
> > hw/core/machine.c | 181
> > +++++++++++++++++++++++++++++++++++++++++++++++++++
> > include/hw/boards.h | 5 ++
> > vl.c | 123 +++++++++++++++-------------------
> > 3 files changed, 240 insertions(+), 69 deletions(-)
> >
> > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > index cbba679..f73b810 100644
> > --- a/hw/core/machine.c
> > +++ b/hw/core/machine.c
> > @@ -12,6 +12,10 @@
> >
> > #include "hw/boards.h"
> > #include "qapi/visitor.h"
> > +#include "qemu/error-report.h"
> > +#include "hw/xen/xen.h"
> > +
> > +static const ram_addr_t default_ram_size = 128 * 1024 * 1024;
> >
> > static char *machine_get_accel(Object *obj, Error **errp)
> > {
> > @@ -235,8 +239,118 @@ static void machine_set_firmware(Object *obj, const
> > char *value, Error **errp)
> > ms->firmware = g_strdup(value);
> > }
> >
> > +static void machine_get_ram_size(Object *obj, Visitor *v,
> > + void *opaque, const char *name,
> > + Error **errp)
> > +{
> > + MachineState *ms = MACHINE(obj);
> > + int64_t value = ms->ram_size;
> > +
> > + visit_type_int(v, &value, name, errp);
> > +}
> > +
> > +static void machine_set_ram_size(Object *obj, Visitor *v,
> > + void *opaque, const char *name,
> > + Error **errp)
> > +{
> > + MachineState *ms = MACHINE(obj);
> > + Error *error = NULL;
> > + int64_t value;
> > +
> > + visit_type_int(v, &value, name, &error);
> > + if (error) {
> > + goto out;
> > + }
> > +
> > + value = QEMU_ALIGN_UP(value, 8192);
> > + ms->ram_size = value;
> > + if (ms->ram_size != value) {
> > + error_setg(&error, "ram size too large");
> > + goto out;
> > + }
> > +
> > + if (!ms->ram_size) {
> > + error_setg(&error, "ram size can't be 0");
> > + }
> > +out:
> > + if (error) {
> > + error_propagate(errp, error);
> > + }
> > +}
> > +
> > +static void machine_get_maxram_size(Object *obj, Visitor *v,
> > + void *opaque, const char *name,
> > + Error **errp)
> > +{
> > + MachineState *ms = MACHINE(obj);
> > + int64_t value = ms->maxram_size;
> > +
> > + visit_type_int(v, &value, name, errp);
> > +}
> > +
> > +static void machine_set_maxram_size(Object *obj, Visitor *v,
> > + void *opaque, const char *name,
> > + Error **errp)
> > +{
> > + MachineState *ms = MACHINE(obj);
> > + Error *error = NULL;
> > + int64_t value;
> > +
> > + visit_type_int(v, &value, name, &error);
> > + if (error) {
> > + goto out;
> > + }
> > +
> > + ms->maxram_size = value;
> > + if (ms->maxram_size != value) {
> > + error_setg(&error, "maxmem is too large");
> > + goto out;
> > + }
> > +
> > + if (!ms->maxram_size) {
> > + error_setg(&error, "maxmem can't be 0");
> > + }
> > +out:
> > + if (error) {
> > + error_propagate(errp, error);
> > + }
> > +}
> > +
> > +static void machine_get_ram_slots(Object *obj, Visitor *v,
> > + void *opaque, const char *name,
> > + Error **errp)
> > +{
> > + MachineState *ms = MACHINE(obj);
> > + int64_t value = ms->ram_slots;
> > +
> > + visit_type_int(v, &value, name, errp);
> > +}
> > +
> > +static void machine_set_ram_slots(Object *obj, Visitor *v,
> > + void *opaque, const char *name,
> > + Error **errp)
> > +{
> > + MachineState *ms = MACHINE(obj);
> > + Error *error = NULL;
> > + int64_t value;
> > +
> > + visit_type_int(v, &value, name, &error);
> > + if (error) {
> > + goto out;
> > + }
> > +
> > + ms->ram_slots = value;
> > +
> > +out:
> > + if (error) {
> > + error_propagate(errp, error);
> > + }
> > +}
> >
>
> Really that's too much boiler plate code for
> each value. Can't we support a "validate" callback from
> standard integer values, to be called after parsing?
machine_set_ram_slots() could be reduced to:
+static void machine_set_ram_slots(Object *obj, Visitor *v,
+ void *opaque, const char *name,
+ Error **errp)
+{
+ MachineState *ms = MACHINE(obj);
+
+ visit_type_int(v, &value, name, errp);
+}
maxram_size/ram_size setters could be reduced ~11 LOC moving common
parts to a helper.
But generic property validator is probably out of scope for this
series and it won't help much in this case.
>
>
> > static void machine_initfn(Object *obj)
> > {
> > + MachineState *ms = MACHINE(obj);
> > +
> > object_property_add_str(obj, "accel",
> > machine_get_accel, machine_set_accel, NULL);
> > object_property_add_bool(obj, "kernel_irqchip",
> > @@ -274,6 +388,20 @@ static void machine_initfn(Object *obj)
> > object_property_add_bool(obj, "usb", machine_get_usb, machine_set_usb,
> > NULL);
> > object_property_add_str(obj, "firmware",
> > machine_get_firmware, machine_set_firmware,
> > NULL);
> > +
> > + ms->ram_size = default_ram_size;
> > + object_property_add(obj, MACHINE_MEMORY_SIZE_OPT, "int",
> > + machine_get_ram_size,
> > + machine_set_ram_size,
> > + NULL, NULL, NULL);
> > + object_property_add(obj, MACHINE_MAXMEMORY_SIZE_OPT, "int",
> > + machine_get_maxram_size,
> > + machine_set_maxram_size,
> > + NULL, NULL, NULL);
> > + object_property_add(obj, MACHINE_MEMORY_SLOTS_OPT, "int",
> > + machine_get_ram_slots,
> > + machine_set_ram_slots,
> > + NULL, NULL, NULL);
> > }
> >
> > static void machine_finalize(Object *obj)
> > @@ -290,11 +418,64 @@ static void machine_finalize(Object *obj)
> > g_free(ms->firmware);
> > }
> >
> > +static void machine_pre_init(MachineState *ms, Error **errp)
> > +{
> > + Error *error = NULL;
> > +
> > + if ((ms->ram_size > ms->maxram_size) && ms->maxram_size) {
> > + error_setg(&error, "invalid \"maxmem\" value (%" RAM_ADDR_FMT
> > + ") < initial memory (%" RAM_ADDR_FMT ")",
> > + ms->maxram_size, ms->ram_size);
> > + goto out;
> > + }
> > +
> > + if ((ms->ram_size < ms->maxram_size) && !ms->ram_slots) {
> > + error_setg(&error, "invalid \"maxmem\" value (%" RAM_ADDR_FMT ") "
> > + "more than initial memory (%" PRIu64 ") but "
> > + "no hotplug slots where specified",
> > + ms->maxram_size, ms->ram_size);
> > + goto out;
> > + }
> > +
> > + if ((ms->ram_size == ms->maxram_size) && ms->ram_slots) {
>
> ugly () within conditions, not really needed.
>
> > + error_setg(&error, "invalid \"maxmem\" value (%" RAM_ADDR_FMT ") "
> > + "it must be more than initial memory if hotplug slots >
> > 0",
> > + ms->maxram_size);
> > + goto out;
> > + }
> > +
> > + if (!ms->maxram_size && !ms->ram_slots) {
> > + ms->maxram_size = ms->ram_size;
> > + }
> > +
> > + if (!xen_enabled()) {
> > + /* On 32-bit hosts, QEMU is limited by virtual address space */
> > + if (HOST_LONG_BITS == 32 && ((ms->ram_size > (2047 << 20)) ||
> > + (ms->maxram_size > (2047 << 20)))) {
> > + error_setg(&error, "at most 2047 MB RAM can be simulated\n");
> > + goto out;
> > + }
> > + }
> > +
> > +out:
> > + if (error) {
> > + error_propagate(errp, error);
> > + }
> > +}
> > +
> > +static void machine_class_init(ObjectClass *oc, void *data)
> > +{
> > + MachineClass *mc = MACHINE_CLASS(oc);
> > +
> > + mc->instance_pre_init = machine_pre_init;
> > +}
> > +
> > 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 605a970..1b980c5 100644
> > --- a/include/hw/boards.h
> > +++ b/include/hw/boards.h
> > @@ -81,6 +81,7 @@ struct MachineClass {
> > const char *desc;
> >
> > void (*init)(MachineState *state);
> > + void (*instance_pre_init)(MachineState *state, Error **errp);
> > void (*reset)(void);
> > void (*hot_add_cpu)(const int64_t id, Error **errp);
> > int (*kvm_type)(const char *arg);
> > @@ -134,4 +135,8 @@ struct MachineState {
> > const char *cpu_model;
> > };
> >
> > +#define MACHINE_MEMORY_SIZE_OPT "memory-size"
> > +#define MACHINE_MEMORY_SLOTS_OPT "memory-slots"
> > +#define MACHINE_MAXMEMORY_SIZE_OPT "maxmem"
> > +
> > #endif
> > diff --git a/vl.c b/vl.c
> > index 0a39c93..3ed3582 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -119,8 +119,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
> >
> > @@ -2928,10 +2926,8 @@ int main(int argc, char **argv, char **envp)
> > const char *trace_events = NULL;
> > const char *trace_file = NULL;
> > Error *local_err = NULL;
> > - const ram_addr_t default_ram_size = (ram_addr_t)DEFAULT_RAM_SIZE *
> > - 1024 * 1024;
> > - ram_addr_t maxram_size = default_ram_size;
> > - uint64_t ram_slots = 0;
> > + const char *maxram_size_str = NULL;
> > + const char *ram_slots_str = NULL;
> > FILE *vmstate_dump_file = NULL;
> >
> > atexit(qemu_run_exit_notifiers);
> > @@ -2978,7 +2974,7 @@ int main(int argc, char **argv, char **envp)
> > module_call_init(MODULE_INIT_MACHINE);
> > machine_class = find_default_machine();
> > cpu_model = NULL;
> > - ram_size = default_ram_size;
> > + ram_size = 0;
> > snapshot = 0;
> > cyls = heads = secs = 0;
> > translation = BIOS_ATA_TRANSLATION_AUTO;
> > @@ -3269,7 +3265,6 @@ int main(int argc, char **argv, char **envp)
> > case QEMU_OPTION_m: {
> > uint64_t sz;
> > const char *mem_str;
> > - const char *maxmem_str, *slots_str;
> >
> > opts = qemu_opts_parse(qemu_find_opts("memory"),
> > optarg, 1);
> > @@ -3287,7 +3282,7 @@ int main(int argc, char **argv, char **envp)
> > exit(EXIT_FAILURE);
> > }
> >
> > - sz = qemu_opt_get_size(opts, "size", ram_size);
> > + sz = qemu_opt_get_size(opts, "size", 0);
> >
> > /* Fix up legacy suffix-less format */
> > if (g_ascii_isdigit(mem_str[strlen(mem_str) - 1])) {
> > @@ -3301,54 +3296,12 @@ int main(int argc, char **argv, char **envp)
> > }
> >
> > /* backward compatibility behaviour for case "-m 0" */
> > - if (sz == 0) {
> > - sz = default_ram_size;
> > - }
> > -
> > - sz = QEMU_ALIGN_UP(sz, 8192);
> > - ram_size = sz;
> > - if (ram_size != sz) {
> > - error_report("ram size too large");
> > - exit(EXIT_FAILURE);
> > + if (sz != 0) {
> > + ram_size = sz;
> > }
> >
> > - maxmem_str = qemu_opt_get(opts, "maxmem");
> > - slots_str = qemu_opt_get(opts, "slots");
> > - if (maxmem_str && slots_str) {
> > - uint64_t slots;
> > -
> > - sz = qemu_opt_get_size(opts, "maxmem", 0);
> > - if (sz < ram_size) {
> > - fprintf(stderr, "qemu: invalid -m option value:
> > maxmem "
> > - "(%" PRIu64 ") <= initial memory (%"
> > - RAM_ADDR_FMT ")\n", sz, ram_size);
> > - exit(EXIT_FAILURE);
> > - }
> > -
> > - slots = qemu_opt_get_number(opts, "slots", 0);
> > - if ((sz > ram_size) && !slots) {
> > - fprintf(stderr, "qemu: invalid -m option value:
> > maxmem "
> > - "(%" PRIu64 ") more than initial memory (%"
> > - RAM_ADDR_FMT ") but no hotplug slots where
> > "
> > - "specified\n", sz, ram_size);
> > - exit(EXIT_FAILURE);
> > - }
> > -
> > - if ((sz <= ram_size) && slots) {
> > - fprintf(stderr, "qemu: invalid -m option value: %"
> > - PRIu64 " hotplug slots where specified but
> > "
> > - "maxmem (%" PRIu64 ") <= initial memory (%"
> > - RAM_ADDR_FMT ")\n", slots, sz, ram_size);
> > - exit(EXIT_FAILURE);
> > - }
> > - maxram_size = sz;
> > - ram_slots = slots;
> > - } else if ((!maxmem_str && slots_str) ||
> > - (maxmem_str && !slots_str)) {
> > - fprintf(stderr, "qemu: invalid -m option value:
> > missing "
> > - "'%s' option\n", slots_str ? "maxmem" :
> > "slots");
> > - exit(EXIT_FAILURE);
> > - }
> > + maxram_size_str = qemu_opt_get(opts, "maxmem");
> > + ram_slots_str = qemu_opt_get(opts, "slots");
> > break;
> > }
> > #ifdef CONFIG_TPM
> > @@ -4203,14 +4156,48 @@ int main(int argc, char **argv, char **envp)
> > exit(1);
> > }
> >
> > - /* store value for the future use */
> > - qemu_opt_set_number(qemu_find_opts_singleton("memory"), "size",
> > ram_size);
> > -
> > if (qemu_opts_foreach(qemu_find_opts("device"), device_help_func,
> > NULL, 0)
> > != 0) {
> > exit(0);
> > }
> >
> > + if (ram_size) {
> > + object_property_set_int(OBJECT(current_machine), ram_size,
> > + MACHINE_MEMORY_SIZE_OPT, &local_err);
> > + if (local_err) {
> > + error_report("%s", error_get_pretty(local_err));
> > + error_free(local_err);
> > + exit(EXIT_FAILURE);
> > + }
> > + }
> > +
> > + if (maxram_size_str) {
> > + uint64_t sz = qemu_opt_get_size(qemu_find_opts_singleton("memory"),
> > + "maxmem", 0);
> > +
> > + parse_option_size("maxmem", maxram_size_str, &sz, &local_err);
> > + if (local_err) {
> > + error_report("%s", error_get_pretty(local_err));
> > + error_free(local_err);
> > + exit(EXIT_FAILURE);
> > + }
> > + object_property_set_int(OBJECT(current_machine), sz,
> > + MACHINE_MAXMEMORY_SIZE_OPT, &local_err);
> > + if (local_err) {
> > + error_report("%s", error_get_pretty(local_err));
> > + error_free(local_err);
> > + exit(EXIT_FAILURE);
> > + }
> > + }
> > +
> > + if (ram_slots_str) {
> > + if (object_set_property(MACHINE_MEMORY_SLOTS_OPT, ram_slots_str,
> > + current_machine)) {
> > + exit(EXIT_FAILURE);
> > + }
> > + }
> > +
> > +
> > machine_opts = qemu_get_machine_opts();
> > if (qemu_opt_foreach(machine_opts, object_set_property,
> > current_machine,
> > 1) < 0) {
> > @@ -4229,6 +4216,9 @@ int main(int argc, char **argv, char **envp)
> > }
> > }
> >
> > + ram_size = object_property_get_int(OBJECT(current_machine),
> > + MACHINE_MEMORY_SIZE_OPT,
> > + &error_abort);
> > machine_opts = qemu_get_machine_opts();
> > kernel_filename = qemu_opt_get(machine_opts, "kernel");
> > initrd_filename = qemu_opt_get(machine_opts, "initrd");
> > @@ -4314,14 +4304,6 @@ int main(int argc, char **argv, char **envp)
> > if (foreach_device_config(DEV_BT, bt_parse))
> > exit(1);
> >
> > - if (!xen_enabled()) {
> > - /* On 32-bit hosts, QEMU is limited by virtual address space */
> > - if (ram_size > (2047 << 20) && HOST_LONG_BITS == 32) {
> > - fprintf(stderr, "qemu: at most 2047 MB RAM can be
> > simulated\n");
> > - exit(1);
> > - }
> > - }
> > -
> > blk_mig_init();
> > ram_mig_init();
> >
> > @@ -4386,12 +4368,15 @@ int main(int argc, char **argv, char **envp)
> >
> > qdev_machine_init();
> >
> > - current_machine->ram_size = ram_size;
> > - current_machine->maxram_size = maxram_size;
> > - current_machine->ram_slots = ram_slots;
> > current_machine->boot_order = boot_order;
> > current_machine->cpu_model = cpu_model;
> >
> > + machine_class->instance_pre_init(current_machine, &local_err);
> > + if (local_err != NULL) {
> > + error_report("%s", error_get_pretty(local_err));
> > + error_free(local_err);
> > + exit(EXIT_FAILURE);
> > + }
> > machine_class->init(current_machine);
> >
> > audio_init();
> > --
> > 1.7.1
>
- Re: [Qemu-devel] [PATCH 2/4] vl.c: use single local_err throughout main(), (continued)
[Qemu-devel] [PATCH 1/4] switch RAM_ADDR_FMT format specifier to print decimal, Igor Mammedov, 2014/06/25
Re: [Qemu-devel] [PATCH 1/4] switch RAM_ADDR_FMT format specifier to print decimal, Peter Maydell, 2014/06/25
[Qemu-devel] [PATCH 3/4] machine: convert ram_size, maxram_size, ram_slots to properties, Igor Mammedov, 2014/06/25
Re: [Qemu-devel] [PATCH 3/4] machine: convert ram_size, maxram_size, ram_slots to properties, Kirill Batuzov, 2014/06/25
[Qemu-devel] [PATCH 4/4] test -m option parameters, Igor Mammedov, 2014/06/25