Skip to contentSkip to conversation

vl.c: fix regression when reading memory size from config file

PATCH In review Marcel Apfelbaum 2 participants 2 messages Jan 11, 2015 – Jan 12, 2015

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 vl.c: fix regression when reading memory size from config file

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.

---
 vl.c | 176 +++++++++++++++++++++++++++++++++++--------------------------------
 1 file changed, 92 insertions(+), 84 deletions(-)
vl.c +92 -84
index adfdbfd..4877a47 100644
@@ -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();
Signature · 1 line
2.1.0
#1 lore raw
Quoted 224 lines · Marcel Apfelbaum

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.

---
 vl.c | 176 +++++++++++++++++++++++++++++++++++--------------------------------
 1 file changed, 92 insertions(+), 84 deletions(-)
vl.c +89 -84
index adfdbfd..4877a47 100644
@@ -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) {

vl.c int main(int argc, char **argv, char **envp) · line 3765

Quoted 9 lines
vl.c +3 -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