Introduce qemu_get_boot_opts()
v3 cover Introduce qemu_get_boot_opts()
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(-)
Signature · 1 line
1.9.2.1.g06c4abd
PATCH v3 1/3 vl.c: Add qemu_get_boot_opts()
Same basic idea as qemu_get_machine_opts().
---
changed since v2:
Use qemu_get_opts_singleton(Pao review)
include/sysemu/sysemu.h | 1 + vl.c | 11 +++++++++++ 2 files changed, 12 insertions(+)
include/sysemu/sysemu.h +1 -0
@@ -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);
vl.c +11 -0
@@ -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;
Signature · 1 line
1.9.2.1.g06c4abd
PATCH v3 2/3 vl.c: Use qemu_get_boot_opts
In reply to Peter CrosthwaiteTo simplfiy and make consistent with surrounding code using qemu_get_machine_opts(). Create a new local variable name boot_opts for consistency as well.
---
vl.c | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-)
vl.c +19 -20
@@ -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 ="" ; }
Signature · 1 line
1.9.2.1.g06c4abd
Re: [Qemu-devel] [PATCH v3 2/3] vl.c: Use qemu_get_boot_opts
Quoted 1 line · Peter Crosthwaite
To simplfiy and make consistent with surrounding code using
simplify
Quoted 2 lines
qemu_get_machine_opts(). Create a new local variable nameboot_optsfor consistency as well.
I'd stick to opts, because its use is local, unlike machine_opts's.
Quoted 21 lines
Signed-off-by Peter Crosthwaite <[email protected]>---
vl.c | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-)vl.c +1
-1@@ -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;
vl.c int main(int argc, char **argv, char **envp) · line 3002
Quoted 3 lines
vl.c +0
-0@@ -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.
Quoted 41 lines
vl.c +13
-19+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);
vl.c int main(int argc, char **argv, char **envp) · line 4272
Quoted 10 lines
vl.c +5
-0+ 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.
PATCH v3 3/3 nvram: fw_cfg: Fix -boot options in nvram/fw_cfg
In reply to Peter CrosthwaiteMultiple -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.
---
changed from v2:
Taken more commit message from 36ad0e9 (Markus Review)
hw/nvram/fw_cfg.c | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-)
hw/nvram/fw_cfg.c +16 -20
@@ -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 ) {
Signature · 1 line
1.9.2.1.g06c4abd
Re: [Qemu-devel] [PATCH v3 3/3] nvram: fw_cfg: Fix -boot options in nvram/fw_cfg
Quoted 70 lines · Peter Crosthwaite
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 <[email protected]>---
changed from v2:
Taken more commit message from 36ad0e9 (Markus Review)hw/nvram/fw_cfg.c | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-)hw/nvram/fw_cfg.c +10
-19@@ -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 );
hw/nvram/fw_cfg.c static void fw_cfg_reboot(FWCfgState *s) · line 200
Quoted 10 lines
hw/nvram/fw_cfg.c +6
-1- } +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.