* [Qemu-devel] [PATCH v3 0/3] Introduce qemu_get_boot_opts() @ 2014-05-08 5:35 Peter Crosthwaite 2014-05-08 5:35 ` [Qemu-devel] [PATCH v3 1/3] vl.c: Add qemu_get_boot_opts() Peter Crosthwaite ` (2 more replies) 0 siblings, 3 replies; 6+ messages in thread From: Peter Crosthwaite @ 2014-05-08 5:35 UTC (permalink / raw) To: qemu-devel; +Cc: armbru Hi Markus, This series introduces qemu_get_boot_opts(), in much the same way as was done for qemu_get_machine_opts(). As usual, I have out-of-scope and out-of-tree usages :) But P3 does clean up the three existing instances of the long-and-awkward form of this query and makes the one in vl.c consistent with an immediately surrounding qemu_get_machine_opts(). changed since v2: Rebase for qemu_get_opts_singleton (Pao review). Elaborated P3 commit message (Markus review). changed since v1: Fix nvram usages as well (Markus review). Regards, Peter Peter Crosthwaite (3): vl.c: Add qemu_get_boot_opts() vl.c: Use qemu_get_boot_opts nvram: fw_cfg: Fix -boot options in nvram/fw_cfg hw/nvram/fw_cfg.c | 36 ++++++++++++++++------------------- include/sysemu/sysemu.h | 1 + vl.c | 50 +++++++++++++++++++++++++++++-------------------- 3 files changed, 47 insertions(+), 40 deletions(-) -- 1.9.2.1.g06c4abd ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH v3 1/3] vl.c: Add qemu_get_boot_opts() 2014-05-08 5:35 [Qemu-devel] [PATCH v3 0/3] Introduce qemu_get_boot_opts() Peter Crosthwaite @ 2014-05-08 5:35 ` Peter Crosthwaite 2014-05-08 5:36 ` [Qemu-devel] [PATCH v3 2/3] vl.c: Use qemu_get_boot_opts Peter Crosthwaite 2014-05-08 5:37 ` [Qemu-devel] [PATCH v3 3/3] nvram: fw_cfg: Fix -boot options in nvram/fw_cfg Peter Crosthwaite 2 siblings, 0 replies; 6+ messages in thread From: Peter Crosthwaite @ 2014-05-08 5:35 UTC (permalink / raw) To: qemu-devel; +Cc: armbru Same basic idea as qemu_get_machine_opts(). Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> --- changed since v2: Use qemu_get_opts_singleton(Pao review) include/sysemu/sysemu.h | 1 + vl.c | 11 +++++++++++ 2 files changed, 12 insertions(+) diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h index ba5c7f8..d41748d 100644 --- a/include/sysemu/sysemu.h +++ b/include/sysemu/sysemu.h @@ -198,6 +198,7 @@ char *get_boot_devices_list(size_t *size, bool ignore_suffixes); DeviceState *get_boot_device(uint32_t position); QemuOpts *qemu_get_machine_opts(void); +QemuOpts *qemu_get_boot_opts(void); bool usb_enabled(bool default_usb); diff --git a/vl.c b/vl.c index 73e0661..10a0c59 100644 --- a/vl.c +++ b/vl.c @@ -534,6 +534,17 @@ QemuOpts *qemu_get_machine_opts(void) return qemu_find_opts_singleton("machine"); } +/** + * Get boot options + * + * Returns: boot options (never null). + */ + +QemuOpts *qemu_get_boot_opts(void) +{ + return qemu_find_opts_singleton("boot-opts"); +} + const char *qemu_get_vm_name(void) { return qemu_name; -- 1.9.2.1.g06c4abd ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH v3 2/3] vl.c: Use qemu_get_boot_opts 2014-05-08 5:35 [Qemu-devel] [PATCH v3 0/3] Introduce qemu_get_boot_opts() Peter Crosthwaite 2014-05-08 5:35 ` [Qemu-devel] [PATCH v3 1/3] vl.c: Add qemu_get_boot_opts() Peter Crosthwaite @ 2014-05-08 5:36 ` Peter Crosthwaite 2014-05-15 18:01 ` Markus Armbruster 2014-05-08 5:37 ` [Qemu-devel] [PATCH v3 3/3] nvram: fw_cfg: Fix -boot options in nvram/fw_cfg Peter Crosthwaite 2 siblings, 1 reply; 6+ messages in thread From: Peter Crosthwaite @ 2014-05-08 5:36 UTC (permalink / raw) To: qemu-devel; +Cc: armbru To simplfiy and make consistent with surrounding code using qemu_get_machine_opts(). Create a new local variable name boot_opts for consistency as well. Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> --- vl.c | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/vl.c b/vl.c index 10a0c59..6d670fe 100644 --- a/vl.c +++ b/vl.c @@ -2971,7 +2971,7 @@ int main(int argc, char **argv, char **envp) const char *boot_order; DisplayState *ds; int cyls, heads, secs, translation; - QemuOpts *hda_opts = NULL, *opts, *machine_opts; + QemuOpts *hda_opts = NULL, *opts, *machine_opts, *boot_opts; QemuOptsList *olist; int optind; const char *optarg; @@ -3000,6 +3000,9 @@ int main(int argc, char **argv, char **envp) const ram_addr_t default_ram_size = (ram_addr_t)DEFAULT_RAM_SIZE * 1024 * 1024; + char *normal_boot_order; + const char *order, *once; + atexit(qemu_run_exit_notifiers); error_set_progname(argv[0]); qemu_init_exec_dir(argv[0]); @@ -4245,29 +4248,25 @@ int main(int argc, char **argv, char **envp) bios_name = qemu_opt_get(machine_opts, "firmware"); boot_order = machine_class->default_boot_order; - opts = qemu_opts_find(qemu_find_opts("boot-opts"), NULL); - if (opts) { - char *normal_boot_order; - const char *order, *once; - - order = qemu_opt_get(opts, "order"); - if (order) { - validate_bootdevices(order); - boot_order = order; - } + boot_opts = qemu_get_boot_opts(); - once = qemu_opt_get(opts, "once"); - if (once) { - validate_bootdevices(once); - normal_boot_order = g_strdup(boot_order); - boot_order = once; - qemu_register_reset(restore_boot_order, normal_boot_order); - } + order = qemu_opt_get(boot_opts, "order"); + if (order) { + validate_bootdevices(order); + boot_order = order; + } - boot_menu = qemu_opt_get_bool(opts, "menu", boot_menu); - boot_strict = qemu_opt_get_bool(opts, "strict", false); + once = qemu_opt_get(boot_opts, "once"); + if (once) { + validate_bootdevices(once); + normal_boot_order = g_strdup(boot_order); + boot_order = once; + qemu_register_reset(restore_boot_order, normal_boot_order); } + boot_menu = qemu_opt_get_bool(boot_opts, "menu", boot_menu); + boot_strict = qemu_opt_get_bool(boot_opts, "strict", false); + if (!kernel_cmdline) { kernel_cmdline = ""; } -- 1.9.2.1.g06c4abd ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v3 2/3] vl.c: Use qemu_get_boot_opts 2014-05-08 5:36 ` [Qemu-devel] [PATCH v3 2/3] vl.c: Use qemu_get_boot_opts Peter Crosthwaite @ 2014-05-15 18:01 ` Markus Armbruster 0 siblings, 0 replies; 6+ messages in thread From: Markus Armbruster @ 2014-05-15 18:01 UTC (permalink / raw) To: Peter Crosthwaite; +Cc: qemu-devel Peter Crosthwaite <peter.crosthwaite@xilinx.com> writes: > To simplfiy and make consistent with surrounding code using simplify > qemu_get_machine_opts(). Create a new local variable name boot_opts > for consistency as well. I'd stick to opts, because its use is local, unlike machine_opts's. > > Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> > --- > > vl.c | 39 +++++++++++++++++++-------------------- > 1 file changed, 19 insertions(+), 20 deletions(-) > > diff --git a/vl.c b/vl.c > index 10a0c59..6d670fe 100644 > --- a/vl.c > +++ b/vl.c > @@ -2971,7 +2971,7 @@ int main(int argc, char **argv, char **envp) > const char *boot_order; > DisplayState *ds; > int cyls, heads, secs, translation; > - QemuOpts *hda_opts = NULL, *opts, *machine_opts; > + QemuOpts *hda_opts = NULL, *opts, *machine_opts, *boot_opts; > QemuOptsList *olist; > int optind; > const char *optarg; > @@ -3000,6 +3000,9 @@ int main(int argc, char **argv, char **envp) > const ram_addr_t default_ram_size = (ram_addr_t)DEFAULT_RAM_SIZE * > 1024 * 1024; > Unwanted blank line above. > + char *normal_boot_order; > + const char *order, *once; > + > atexit(qemu_run_exit_notifiers); > error_set_progname(argv[0]); > qemu_init_exec_dir(argv[0]); > @@ -4245,29 +4248,25 @@ int main(int argc, char **argv, char **envp) > bios_name = qemu_opt_get(machine_opts, "firmware"); > > boot_order = machine_class->default_boot_order; > - opts = qemu_opts_find(qemu_find_opts("boot-opts"), NULL); > - if (opts) { > - char *normal_boot_order; > - const char *order, *once; > - > - order = qemu_opt_get(opts, "order"); > - if (order) { > - validate_bootdevices(order); > - boot_order = order; > - } > + boot_opts = qemu_get_boot_opts(); > > - once = qemu_opt_get(opts, "once"); > - if (once) { > - validate_bootdevices(once); > - normal_boot_order = g_strdup(boot_order); > - boot_order = once; > - qemu_register_reset(restore_boot_order, normal_boot_order); > - } > + order = qemu_opt_get(boot_opts, "order"); > + if (order) { > + validate_bootdevices(order); > + boot_order = order; > + } > > - boot_menu = qemu_opt_get_bool(opts, "menu", boot_menu); > - boot_strict = qemu_opt_get_bool(opts, "strict", false); > + once = qemu_opt_get(boot_opts, "once"); > + if (once) { > + validate_bootdevices(once); > + normal_boot_order = g_strdup(boot_order); > + boot_order = once; > + qemu_register_reset(restore_boot_order, normal_boot_order); > } > > + boot_menu = qemu_opt_get_bool(boot_opts, "menu", boot_menu); > + boot_strict = qemu_opt_get_bool(boot_opts, "strict", false); > + > if (!kernel_cmdline) { > kernel_cmdline = ""; > } I'd be tempted to try putting this into its own function while I'm at it. Correct the typo and the blank line, and you can have my R-by. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH v3 3/3] nvram: fw_cfg: Fix -boot options in nvram/fw_cfg 2014-05-08 5:35 [Qemu-devel] [PATCH v3 0/3] Introduce qemu_get_boot_opts() Peter Crosthwaite 2014-05-08 5:35 ` [Qemu-devel] [PATCH v3 1/3] vl.c: Add qemu_get_boot_opts() Peter Crosthwaite 2014-05-08 5:36 ` [Qemu-devel] [PATCH v3 2/3] vl.c: Use qemu_get_boot_opts Peter Crosthwaite @ 2014-05-08 5:37 ` Peter Crosthwaite 2014-05-15 18:03 ` Markus Armbruster 2 siblings, 1 reply; 6+ messages in thread From: Peter Crosthwaite @ 2014-05-08 5:37 UTC (permalink / raw) To: qemu-devel; +Cc: armbru Multiple -boot options with the same ID are merged. All but the one without an ID are to be silently ignored. In other places, we query boot options with qemu_get_boot_opts(). This is correct. In this instance, we instead query whatever options come first in the list. This is wrong. When the -boot processed first happens to have an ID, options are taken from that ID, and the ones specified without ID are silently ignored. Use qemu_get_boot_opts() to fix these bugs. This change is similar to and based on 36ad0e9. We also take to opportunity to remove the now unneeded null boot-opts conditional, removing a level of indentation on usage code. Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> --- changed from v2: Taken more commit message from 36ad0e9 (Markus Review) hw/nvram/fw_cfg.c | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c index 282341a..8537669 100644 --- a/hw/nvram/fw_cfg.c +++ b/hw/nvram/fw_cfg.c @@ -125,18 +125,16 @@ static void fw_cfg_bootsplash(FWCfgState *s) const char *temp; /* get user configuration */ - QemuOptsList *plist = qemu_find_opts("boot-opts"); - QemuOpts *opts = QTAILQ_FIRST(&plist->head); - if (opts != NULL) { - temp = qemu_opt_get(opts, "splash"); - if (temp != NULL) { - boot_splash_filename = temp; - } - temp = qemu_opt_get(opts, "splash-time"); - if (temp != NULL) { - p = (char *)temp; - boot_splash_time = strtol(p, (char **)&p, 10); - } + QemuOpts *opts = qemu_get_boot_opts(); + + temp = qemu_opt_get(opts, "splash"); + if (temp != NULL) { + boot_splash_filename = temp; + } + temp = qemu_opt_get(opts, "splash-time"); + if (temp != NULL) { + p = (char *)temp; + boot_splash_time = strtol(p, (char **)&p, 10); } /* insert splash time if user configurated */ @@ -191,14 +189,12 @@ static void fw_cfg_reboot(FWCfgState *s) const char *temp; /* get user configuration */ - QemuOptsList *plist = qemu_find_opts("boot-opts"); - QemuOpts *opts = QTAILQ_FIRST(&plist->head); - if (opts != NULL) { - temp = qemu_opt_get(opts, "reboot-timeout"); - if (temp != NULL) { - p = (char *)temp; - reboot_timeout = strtol(p, (char **)&p, 10); - } + QemuOpts *opts = qemu_get_boot_opts(); + + temp = qemu_opt_get(opts, "reboot-timeout"); + if (temp != NULL) { + p = (char *)temp; + reboot_timeout = strtol(p, (char **)&p, 10); } /* validate the input */ if (reboot_timeout > 0xffff) { -- 1.9.2.1.g06c4abd ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v3 3/3] nvram: fw_cfg: Fix -boot options in nvram/fw_cfg 2014-05-08 5:37 ` [Qemu-devel] [PATCH v3 3/3] nvram: fw_cfg: Fix -boot options in nvram/fw_cfg Peter Crosthwaite @ 2014-05-15 18:03 ` Markus Armbruster 0 siblings, 0 replies; 6+ messages in thread From: Markus Armbruster @ 2014-05-15 18:03 UTC (permalink / raw) To: Peter Crosthwaite; +Cc: qemu-devel Peter Crosthwaite <peter.crosthwaite@xilinx.com> writes: > Multiple -boot options with the same ID are merged. All but the > one without an ID are to be silently ignored. > > In other places, we query boot options with qemu_get_boot_opts(). > This is correct. > > In this instance, we instead query whatever options come first in the > list. This is wrong. When the -boot processed first happens to > have an ID, options are taken from that ID, and the ones specified > without ID are silently ignored. > > Use qemu_get_boot_opts() to fix these bugs. > > This change is similar to and based on 36ad0e9. > > We also take to opportunity to remove the now unneeded null boot-opts > conditional, removing a level of indentation on usage code. > > Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> > --- > changed from v2: > Taken more commit message from 36ad0e9 (Markus Review) > > hw/nvram/fw_cfg.c | 36 ++++++++++++++++-------------------- > 1 file changed, 16 insertions(+), 20 deletions(-) > > diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c > index 282341a..8537669 100644 > --- a/hw/nvram/fw_cfg.c > +++ b/hw/nvram/fw_cfg.c > @@ -125,18 +125,16 @@ static void fw_cfg_bootsplash(FWCfgState *s) > const char *temp; > > /* get user configuration */ > - QemuOptsList *plist = qemu_find_opts("boot-opts"); > - QemuOpts *opts = QTAILQ_FIRST(&plist->head); > - if (opts != NULL) { > - temp = qemu_opt_get(opts, "splash"); > - if (temp != NULL) { > - boot_splash_filename = temp; > - } > - temp = qemu_opt_get(opts, "splash-time"); > - if (temp != NULL) { > - p = (char *)temp; > - boot_splash_time = strtol(p, (char **)&p, 10); > - } > + QemuOpts *opts = qemu_get_boot_opts(); > + > + temp = qemu_opt_get(opts, "splash"); > + if (temp != NULL) { > + boot_splash_filename = temp; > + } > + temp = qemu_opt_get(opts, "splash-time"); > + if (temp != NULL) { > + p = (char *)temp; > + boot_splash_time = strtol(p, (char **)&p, 10); > } > > /* insert splash time if user configurated */ > @@ -191,14 +189,12 @@ static void fw_cfg_reboot(FWCfgState *s) > const char *temp; > > /* get user configuration */ > - QemuOptsList *plist = qemu_find_opts("boot-opts"); > - QemuOpts *opts = QTAILQ_FIRST(&plist->head); > - if (opts != NULL) { > - temp = qemu_opt_get(opts, "reboot-timeout"); > - if (temp != NULL) { > - p = (char *)temp; > - reboot_timeout = strtol(p, (char **)&p, 10); > - } > + QemuOpts *opts = qemu_get_boot_opts(); > + > + temp = qemu_opt_get(opts, "reboot-timeout"); > + if (temp != NULL) { > + p = (char *)temp; > + reboot_timeout = strtol(p, (char **)&p, 10); > } > /* validate the input */ > if (reboot_timeout > 0xffff) { I'd appreciate a follow-up patch making "splash-time" and "reboot-timeout" QEMU_OPT_NUMBER, gotten with qemu_opt_number(). But it's not required to get this one in. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-05-15 18:03 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-05-08 5:35 [Qemu-devel] [PATCH v3 0/3] Introduce qemu_get_boot_opts() Peter Crosthwaite 2014-05-08 5:35 ` [Qemu-devel] [PATCH v3 1/3] vl.c: Add qemu_get_boot_opts() Peter Crosthwaite 2014-05-08 5:36 ` [Qemu-devel] [PATCH v3 2/3] vl.c: Use qemu_get_boot_opts Peter Crosthwaite 2014-05-15 18:01 ` Markus Armbruster 2014-05-08 5:37 ` [Qemu-devel] [PATCH v3 3/3] nvram: fw_cfg: Fix -boot options in nvram/fw_cfg Peter Crosthwaite 2014-05-15 18:03 ` Markus Armbruster
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.