[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PULL 13/27] machine: add mem compound property
From: |
Markus Armbruster |
Subject: |
Re: [PULL 13/27] machine: add mem compound property |
Date: |
Mon, 13 Jun 2022 15:42:24 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) |
Paolo Bonzini <pbonzini@redhat.com> writes:
> Make -m syntactic sugar for a compound property "-machine
> mem.{size,max-size,slots}". The new property does not have
> the magic conversion to megabytes of unsuffixed arguments,
> and also does not understand that "0" means the default size
> (you have to leave it out to get the default). This means
> that we need to convert the QemuOpts by hand to a QDict.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Message-Id: <20220414165300.555321-4-pbonzini@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> hw/core/machine.c | 80 ++++++++++++++++++++++++++++++
> qapi/machine.json | 18 +++++++
> softmmu/vl.c | 123 +++++++++++++++-------------------------------
> 3 files changed, 138 insertions(+), 83 deletions(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 8cea94537d..46b8d0effa 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -523,6 +523,78 @@ static void machine_set_hmat(Object *obj, bool value,
> Error **errp)
> ms->numa_state->hmat_enabled = value;
> }
>
> +static void machine_get_mem(Object *obj, Visitor *v, const char *name,
> + void *opaque, Error **errp)
> +{
> + MachineState *ms = MACHINE(obj);
> + MemorySizeConfiguration mem = {
> + .has_size = true,
> + .size = ms->ram_size,
> + .has_max_size = !!ms->ram_slots,
> + .max_size = ms->maxram_size,
> + .has_slots = !!ms->ram_slots,
> + .slots = ms->ram_slots,
> + };
> + MemorySizeConfiguration *p_mem = &mem;
> +
> + visit_type_MemorySizeConfiguration(v, name, &p_mem, &error_abort);
> +}
> +
> +static void machine_set_mem(Object *obj, Visitor *v, const char *name,
> + void *opaque, Error **errp)
> +{
> + MachineState *ms = MACHINE(obj);
> + MachineClass *mc = MACHINE_GET_CLASS(obj);
> + MemorySizeConfiguration *mem;
> +
> + ERRP_GUARD();
> +
> + if (!visit_type_MemorySizeConfiguration(v, name, &mem, errp)) {
> + return;
> + }
> +
> + if (!mem->has_size) {
> + mem->has_size = true;
> + mem->size = mc->default_ram_size;
> + }
> + mem->size = QEMU_ALIGN_UP(mem->size, 8192);
> + if (mc->fixup_ram_size) {
> + mem->size = mc->fixup_ram_size(mem->size);
> + }
> + if ((ram_addr_t)mem->size != mem->size) {
> + error_setg(errp, "ram size too large");
> + goto out_free;
> + }
> +
> + if (mem->has_max_size) {
> + if (mem->max_size < mem->size) {
> + error_setg(errp, "invalid value of maxmem: "
> + "maximum memory size (0x%" PRIx64 ") must be at least
> "
> + "the initial memory size (0x%" PRIx64 ")",
> + mem->max_size, mem->size);
> + goto out_free;
> + }
> + if (mem->has_slots && mem->slots && mem->max_size == mem->size) {
> + error_setg(errp, "invalid value of maxmem: "
> + "memory slots were specified but maximum memory size "
> + "(0x%" PRIx64 ") is equal to the initial memory size "
> + "(0x%" PRIx64 ")", mem->max_size, mem->size);
> + goto out_free;
> + }
> + ms->maxram_size = mem->max_size;
> + } else {
> + if (mem->has_slots) {
> + error_setg(errp, "slots specified but no max-size");
> + goto out_free;
> + }
> + ms->maxram_size = mem->size;
> + }
> + ms->ram_size = mem->size;
> + ms->ram_slots = mem->has_slots ? mem->slots : 0;
> +out_free:
> + qapi_free_MemorySizeConfiguration(mem);
> +}
> +
> static char *machine_get_nvdimm_persistence(Object *obj, Error **errp)
> {
> MachineState *ms = MACHINE(obj);
> @@ -953,6 +1025,12 @@ static void machine_class_init(ObjectClass *oc, void
> *data)
> object_class_property_set_description(oc, "memory-backend",
> "Set RAM backend"
> "Valid value is ID of hostmem
> based backend");
> +
> + object_class_property_add(oc, "memory", "MemorySizeConfiguration",
> + machine_get_mem, machine_set_mem,
> + NULL, NULL);
> + object_class_property_set_description(oc, "memory",
> + "Memory size configuration");
> }
>
> static void machine_class_base_init(ObjectClass *oc, void *data)
> @@ -983,6 +1061,8 @@ static void machine_initfn(Object *obj)
> ms->mem_merge = true;
> ms->enable_graphics = true;
> ms->kernel_cmdline = g_strdup("");
> + ms->ram_size = mc->default_ram_size;
> + ms->maxram_size = mc->default_ram_size;
>
> if (mc->nvdimm_supported) {
> Object *obj = OBJECT(ms);
> diff --git a/qapi/machine.json b/qapi/machine.json
> index e3dcf5a119..92480d4044 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -1614,3 +1614,21 @@
> ##
> { 'enum': 'SmbiosEntryPointType',
> 'data': [ '32', '64' ] }
> +
> +##
> +# @MemorySizeConfiguration:
> +#
> +# Schema for memory size configuration.
> +#
> +# @size: memory size in bytes
> +#
> +# @max-size: maximum hotpluggable memory size in bytes
> +#
> +# @slots: number of available memory slots for hotplug
> +#
> +# Since: 7.1
> +##
> +{ 'struct': 'MemorySizeConfiguration', 'data': {
> + '*size': 'size',
> + '*max-size': 'size',
> + '*slots': 'uint64' } }
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 13ae31e92f..65a665e0bc 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -159,11 +159,10 @@ static const char *mem_path;
> static const char *incoming;
> static const char *loadvm;
> static const char *accelerators;
> +static bool have_custom_ram_size;
> static QDict *machine_opts_dict;
> static QTAILQ_HEAD(, ObjectOption) object_opts =
> QTAILQ_HEAD_INITIALIZER(object_opts);
> static QTAILQ_HEAD(, DeviceOption) device_opts =
> QTAILQ_HEAD_INITIALIZER(device_opts);
> -static ram_addr_t maxram_size;
> -static uint64_t ram_slots;
> static int display_remote;
> static int snapshot;
> static bool preconfig_requested;
> @@ -171,7 +170,6 @@ static QemuPluginList plugin_list =
> QTAILQ_HEAD_INITIALIZER(plugin_list);
> static BlockdevOptionsQueue bdo_queue = QSIMPLEQ_HEAD_INITIALIZER(bdo_queue);
> static bool nographic = false;
> static int mem_prealloc; /* force preallocation of physical target memory */
> -static ram_addr_t ram_size;
> static const char *vga_model = NULL;
> static DisplayOptions dpy;
> static int num_serial_hds;
> @@ -1736,6 +1734,7 @@ static void keyval_dashify(QDict *qdict, Error **errp)
> static void qemu_apply_legacy_machine_options(QDict *qdict)
> {
> const char *value;
> + QObject *prop;
>
> keyval_dashify(qdict, &error_fatal);
>
> @@ -1768,6 +1767,13 @@ static void qemu_apply_legacy_machine_options(QDict
> *qdict)
> false);
> qdict_del(qdict, "kernel-irqchip");
> }
> +
> + prop = qdict_get(qdict, "memory");
> + if (prop) {
> + have_custom_ram_size =
> + qobject_type(prop) == QTYPE_QDICT &&
> + qdict_haskey(qobject_to(QDict, prop), "size");
> + }
> }
>
> static void object_option_foreach_add(bool (*type_opt_predicate)(const char
> *))
> @@ -1885,9 +1891,6 @@ static bool object_create_early(const char *type)
> static void qemu_apply_machine_options(QDict *qdict)
> {
> object_set_properties_from_keyval(OBJECT(current_machine), qdict, false,
> &error_fatal);
> - current_machine->ram_size = ram_size;
> - current_machine->maxram_size = maxram_size;
> - current_machine->ram_slots = ram_slots;
>
> if (semihosting_enabled() && !semihosting_get_argc()) {
> /* fall back to the -kernel/-append */
> @@ -1998,12 +2001,6 @@ static void qemu_create_late_backends(void)
> qemu_semihosting_console_init();
> }
>
> -static bool have_custom_ram_size(void)
> -{
> - QemuOpts *opts = qemu_find_opts_singleton("memory");
> - return !!qemu_opt_get_size(opts, "size", 0);
> -}
> -
> static void qemu_resolve_machine_memdev(void)
> {
> if (current_machine->ram_memdev_id) {
> @@ -2018,7 +2015,7 @@ static void qemu_resolve_machine_memdev(void)
> exit(EXIT_FAILURE);
> }
> backend_size = object_property_get_uint(backend, "size",
> &error_abort);
> - if (have_custom_ram_size() && backend_size != ram_size) {
> + if (have_custom_ram_size && backend_size !=
> current_machine->ram_size) {
> error_report("Size specified by -m option must match size of
> "
> "explicitly specified 'memory-backend'
> property");
> exit(EXIT_FAILURE);
> @@ -2028,95 +2025,58 @@ static void qemu_resolve_machine_memdev(void)
> "'-machine memory-backend'");
> exit(EXIT_FAILURE);
> }
> - ram_size = backend_size;
> + current_machine->ram_size = backend_size;
> }
>
> if (!xen_enabled()) {
> /* On 32-bit hosts, QEMU is limited by virtual address space */
> - if (ram_size > (2047 << 20) && HOST_LONG_BITS == 32) {
> + if (current_machine->ram_size > (2047 << 20) && HOST_LONG_BITS ==
> 32) {
> error_report("at most 2047 MB RAM can be simulated");
> exit(1);
> }
> }
> }
>
> -static void set_memory_options(MachineClass *mc)
> +static void parse_memory_options(const char *arg)
> {
> - uint64_t sz;
> + QemuOpts *opts;
> + QDict *dict, *prop;
> const char *mem_str;
> - const ram_addr_t default_ram_size = mc->default_ram_size;
> - QemuOpts *opts = qemu_find_opts_singleton("memory");
> - Location loc;
>
> - loc_push_none(&loc);
> - qemu_opts_loc_restore(opts);
> + opts = qemu_opts_parse_noisily(qemu_find_opts("memory"), arg, true);
> + if (!opts) {
> + exit(EXIT_FAILURE);
> + }
>
> - sz = 0;
> - mem_str = qemu_opt_get(opts, "size");
> - if (mem_str) {
> + prop = qdict_new();
> +
> + if (qemu_opt_get_size(opts, "size", 0) != 0) {
> + mem_str = qemu_opt_get(opts, "size");
> if (!*mem_str) {
> error_report("missing 'size' option value");
> exit(EXIT_FAILURE);
> }
>
> - sz = qemu_opt_get_size(opts, "size", ram_size);
> -
> /* Fix up legacy suffix-less format */
> if (g_ascii_isdigit(mem_str[strlen(mem_str) - 1])) {
> - uint64_t overflow_check = sz;
> -
> - sz *= MiB;
> - if (sz / MiB != overflow_check) {
> - error_report("too large 'size' option value");
> - exit(EXIT_FAILURE);
> - }
> + g_autofree char *mib_str = g_strdup_printf("%sM", mem_str);
> + qdict_put_str(prop, "size", mib_str);
> + } else {
> + qdict_put_str(prop, "size", mem_str);
> }
> }
>
> - /* backward compatibility behaviour for case "-m 0" */
> - if (sz == 0) {
> - sz = default_ram_size;
> - }
> -
> - sz = QEMU_ALIGN_UP(sz, 8192);
> - if (mc->fixup_ram_size) {
> - sz = mc->fixup_ram_size(sz);
> - }
> - ram_size = sz;
> - if (ram_size != sz) {
> - error_report("ram size too large");
> - exit(EXIT_FAILURE);
> - }
> -
> - maxram_size = ram_size;
> -
> if (qemu_opt_get(opts, "maxmem")) {
> - uint64_t slots;
> -
> - sz = qemu_opt_get_size(opts, "maxmem", 0);
> - slots = qemu_opt_get_number(opts, "slots", 0);
> - if (sz < ram_size) {
> - error_report("invalid value of -m option maxmem: "
> - "maximum memory size (0x%" PRIx64 ") must be at
> least "
> - "the initial memory size (0x" RAM_ADDR_FMT ")",
> - sz, ram_size);
> - exit(EXIT_FAILURE);
> - } else if (slots && sz == ram_size) {
> - error_report("invalid value of -m option maxmem: "
> - "memory slots were specified but maximum memory
> size "
> - "(0x%" PRIx64 ") is equal to the initial memory
> size "
> - "(0x" RAM_ADDR_FMT ")", sz, ram_size);
> - exit(EXIT_FAILURE);
> - }
> -
> - maxram_size = sz;
> - ram_slots = slots;
> - } else if (qemu_opt_get(opts, "slots")) {
> - error_report("invalid -m option value: missing 'maxmem' option");
> - exit(EXIT_FAILURE);
> + qdict_put_str(prop, "max-size", qemu_opt_get(opts, "maxmem"));
> + }
> + if (qemu_opt_get(opts, "slots")) {
> + qdict_put_str(prop, "slots", qemu_opt_get(opts, "slots"));
> }
>
> - loc_pop(&loc);
> + dict = qdict_new();
> + qdict_put(dict, "memory", prop);
> + keyval_merge(machine_opts_dict, dict, &error_fatal);
> + qobject_unref(dict);
> }
>
> static void qemu_create_machine(QDict *qdict)
> @@ -2124,8 +2084,6 @@ static void qemu_create_machine(QDict *qdict)
> MachineClass *machine_class = select_machine(qdict, &error_fatal);
> object_set_machine_compat_props(machine_class->compat_props);
>
> - set_memory_options(machine_class);
> -
> current_machine =
> MACHINE(object_new_with_class(OBJECT_CLASS(machine_class)));
> object_property_add_child(object_get_root(), "machine",
> OBJECT(current_machine));
> @@ -2185,7 +2143,8 @@ static bool is_qemuopts_group(const char *group)
> if (g_str_equal(group, "object") ||
> g_str_equal(group, "machine") ||
> g_str_equal(group, "smp-opts") ||
> - g_str_equal(group, "boot-opts")) {
> + g_str_equal(group, "boot-opts") ||
> + g_str_equal(group, "memory")) {
> return false;
> }
> return true;
> @@ -2209,6 +2168,8 @@ static void qemu_record_config_group(const char *group,
> QDict *dict,
> machine_merge_property("smp", dict, &error_fatal);
> } else if (g_str_equal(group, "boot-opts")) {
> machine_merge_property("boot", dict, &error_fatal);
> + } else if (g_str_equal(group, "memory")) {
> + machine_merge_property("memory", dict, &error_fatal);
> } else {
> abort();
> }
> @@ -3009,11 +2970,7 @@ void qemu_init(int argc, char **argv, char **envp)
> exit(0);
> break;
> case QEMU_OPTION_m:
> - opts = qemu_opts_parse_noisily(qemu_find_opts("memory"),
> - optarg, true);
> - if (!opts) {
> - exit(EXIT_FAILURE);
> - }
> + parse_memory_options(optarg);
> break;
> #ifdef CONFIG_TPM
> case QEMU_OPTION_tpmdev:
This appears to change the meaning of
[memory]
size = "1024"
in a -readconfig file from 1024MiB to 8KiB (1024 Bytes rounded up to
8KiB silently).
Aside: the failure mode is nasty: "KVM internal error. Suberror: 1".
Known issue. Or rather known again issue (to me); I thought I had
broken KVM somehow.
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [PULL 13/27] machine: add mem compound property,
Markus Armbruster <=