On 8/5/22 15:40, Markus Armbruster wrote:
+ loc_push_none(&loc);
+ qemu_opts_loc_restore(opts);
+
prop = qdict_new();
if (qemu_opt_get_size(opts, "size", 0) != 0) {
This treats "size=0" like absent size. Before commit ce9d03fb3f, we
instead checked
mem_str = qemu_opt_get(opts, "size");
if (mem_str) {
Makes more sense, doesn't it?
True, on the other hand before commit ce9d03fb3f we handled "-m 0" like
this:
sz = 0;
mem_str = qemu_opt_get(opts, "size");
if (mem_str) {
...
}
if (sz == 0) {
sz = default_ram_size;
}
Now instead, the "-m 0" case results in no qdict_put_str() call at all.
So the code flows better with qemu_opt_get_size(opts, "size", 0).
In addition, using qemu_opt_get_size() is what enables the dead code
removal below, because it generates an error for empty size.
Also, with the new check above, the check below...
mem_str = qemu_opt_get(opts, "size");
if (!*mem_str) {
error_report("missing 'size' option value");
exit(EXIT_FAILURE);
}
... looks dead. We get there only when qemu_opt_get_size() returns
non-zero, which implies a non-blank string.
Makes sense. Separate patch?
static void qemu_create_machine(QDict *qdict)
Commit ce9d03fb3f changed this function's purpose and renamed it from
set_memory_options() to parse_memory_options().
This commit is a partial revert. It doesn't revert the change of name.
Intentional?
Yes, though honestly both are pretty bad names. set_memory_options() is
bad because it's not setting anything, it's just putting the values in a
QDict. I kept parse_*() because it does do a limited amount of parsing
to handle the suffix-less memory size.
Paolo
