From: Paolo Bonzini <pbonzini@redhat.com>
To: Marcel Apfelbaum <marcel@redhat.com>, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] vl.c: fix regression when reading memory size from config file
Date: Mon, 12 Jan 2015 11:17:20 +0100 [thread overview]
Message-ID: <54B39F30.3040308@redhat.com> (raw)
In-Reply-To: <1420972723-19698-1-git-send-email-marcel@redhat.com>
On 11/01/2015 11:38, Marcel Apfelbaum wrote:
> This is happening because an actual logic is performed on the memory
> arguments inside the main's switch, disregarding the config file content.
>
> Solved by extracting the logic on a separate function and calling it
> after the switch.
>
> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
> ---
> vl.c | 176 +++++++++++++++++++++++++++++++++++--------------------------------
> 1 file changed, 92 insertions(+), 84 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index adfdbfd..4877a47 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2644,6 +2644,93 @@ out:
> return 0;
> }
>
> +static void set_memory_options(uint64_t *ram_slots, ram_addr_t *maxram_size)
> +{
> + 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;
> + QemuOpts *opts = qemu_find_opts_singleton("memory");
> +
> + ram_size = default_ram_size;
> + *maxram_size = default_ram_size;
> +
> + mem_str = qemu_opt_get(opts, "size");
> + if (!mem_str) {
> + error_report("invalid -m option, missing 'size' option");
> + exit(EXIT_FAILURE);
> + }
> + 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 <<= 20;
> + if ((sz >> 20) != overflow_check) {
> + error_report("too large 'size' option value");
> + exit(EXIT_FAILURE);
> + }
> + }
> +
> + /* 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);
> + }
> + *maxram_size = ram_size;
> +
> + 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) {
> + error_report("invalid -m option value: maxmem "
> + "(0x%" PRIx64 ") <= initial memory (0x"
> + RAM_ADDR_FMT ")", sz, ram_size);
> + exit(EXIT_FAILURE);
> + }
> +
> + slots = qemu_opt_get_number(opts, "slots", 0);
> + if ((sz > ram_size) && !slots) {
> + error_report("invalid -m option value: maxmem "
> + "(0x%" PRIx64 ") more than initial memory (0x"
> + RAM_ADDR_FMT ") but no hotplug slots where "
> + "specified", sz, ram_size);
> + exit(EXIT_FAILURE);
> + }
> +
> + if ((sz <= ram_size) && slots) {
> + error_report("invalid -m option value: %"
> + PRIu64 " hotplug slots where specified but "
> + "maxmem (0x%" PRIx64 ") <= initial memory (0x"
> + RAM_ADDR_FMT ")", slots, sz, ram_size);
> + exit(EXIT_FAILURE);
> + }
> + *maxram_size = sz;
> + *ram_slots = slots;
> + } else if ((!maxmem_str && slots_str) ||
> + (maxmem_str && !slots_str)) {
> + error_report("invalid -m option value: missing "
> + "'%s' option", slots_str ? "maxmem" : "slots");
> + exit(EXIT_FAILURE);
> + }
> +}
> +
> int main(int argc, char **argv, char **envp)
> {
> int i;
> @@ -2679,9 +2766,7 @@ int main(int argc, char **argv, char **envp)
> };
> const char *trace_events = NULL;
> const char *trace_file = NULL;
> - const ram_addr_t default_ram_size = (ram_addr_t)DEFAULT_RAM_SIZE *
> - 1024 * 1024;
> - ram_addr_t maxram_size = default_ram_size;
> + ram_addr_t maxram_size;
> uint64_t ram_slots = 0;
> FILE *vmstate_dump_file = NULL;
> Error *main_loop_err = NULL;
> @@ -2732,7 +2817,6 @@ 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;
> snapshot = 0;
> cyls = heads = secs = 0;
> translation = BIOS_ATA_TRANSLATION_AUTO;
> @@ -3022,92 +3106,13 @@ int main(int argc, char **argv, char **envp)
> version();
> exit(0);
> break;
> - case QEMU_OPTION_m: {
> - uint64_t sz;
> - const char *mem_str;
> - const char *maxmem_str, *slots_str;
> -
> + case QEMU_OPTION_m:
> opts = qemu_opts_parse(qemu_find_opts("memory"),
> optarg, 1);
> if (!opts) {
> exit(EXIT_FAILURE);
> }
> -
> - mem_str = qemu_opt_get(opts, "size");
> - if (!mem_str) {
> - error_report("invalid -m option, missing 'size' option");
> - exit(EXIT_FAILURE);
> - }
> - 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 <<= 20;
> - if ((sz >> 20) != overflow_check) {
> - error_report("too large 'size' option value");
> - exit(EXIT_FAILURE);
> - }
> - }
> -
> - /* 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);
> - }
> - maxram_size = ram_size;
> -
> - 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) {
> - error_report("invalid -m option value: maxmem "
> - "(0x%" PRIx64 ") <= initial memory (0x"
> - RAM_ADDR_FMT ")", sz, ram_size);
> - exit(EXIT_FAILURE);
> - }
> -
> - slots = qemu_opt_get_number(opts, "slots", 0);
> - if ((sz > ram_size) && !slots) {
> - error_report("invalid -m option value: maxmem "
> - "(0x%" PRIx64 ") more than initial memory (0x"
> - RAM_ADDR_FMT ") but no hotplug slots where "
> - "specified", sz, ram_size);
> - exit(EXIT_FAILURE);
> - }
> -
> - if ((sz <= ram_size) && slots) {
> - error_report("invalid -m option value: %"
> - PRIu64 " hotplug slots where specified but "
> - "maxmem (0x%" PRIx64 ") <= initial memory (0x"
> - RAM_ADDR_FMT ")", slots, sz, ram_size);
> - exit(EXIT_FAILURE);
> - }
> - maxram_size = sz;
> - ram_slots = slots;
> - } else if ((!maxmem_str && slots_str) ||
> - (maxmem_str && !slots_str)) {
> - error_report("invalid -m option value: missing "
> - "'%s' option", slots_str ? "maxmem" : "slots");
> - exit(EXIT_FAILURE);
> - }
> break;
> - }
> #ifdef CONFIG_TPM
> case QEMU_OPTION_tpmdev:
> if (tpm_config_parse(qemu_find_opts("tpmdev"), optarg) < 0) {
> @@ -3752,6 +3757,9 @@ int main(int argc, char **argv, char **envp)
> }
> }
> }
> +
> + set_memory_options(&ram_slots, &maxram_size);
> +
> loc_set_none();
>
> os_daemonize();
>
Looks good, thanks.
Paolo
prev parent reply other threads:[~2015-01-12 10:17 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-01-11 10:38 [Qemu-devel] [PATCH] vl.c: fix regression when reading memory size from config file Marcel Apfelbaum
2015-01-12 10:17 ` Paolo Bonzini [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=54B39F30.3040308@redhat.com \
--to=pbonzini@redhat.com \
--cc=marcel@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).