From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45971) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YLqsJ-00006n-2g for qemu-devel@nongnu.org; Thu, 12 Feb 2015 05:20:00 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YLqsD-0004GD-R5 for qemu-devel@nongnu.org; Thu, 12 Feb 2015 05:19:59 -0500 Received: from mx1.redhat.com ([209.132.183.28]:53422) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YLqsB-0004G5-Rz for qemu-devel@nongnu.org; Thu, 12 Feb 2015 05:19:53 -0500 From: Markus Armbruster References: <1423293616-10736-1-git-send-email-arei.gonglei@huawei.com> <1423293616-10736-3-git-send-email-arei.gonglei@huawei.com> Date: Thu, 12 Feb 2015 11:19:46 +0100 In-Reply-To: <1423293616-10736-3-git-send-email-arei.gonglei@huawei.com> (arei gonglei's message of "Sat, 7 Feb 2015 15:20:14 +0800") Message-ID: <87sieba0u5.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v3 2/4] bootdevice: check boot order argument validation before vm running List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: arei.gonglei@huawei.com Cc: peter.huangpeng@huawei.com, dvaleev@suse.de, qemu-devel@nongnu.org, agraf@suse.de writes: > From: Gonglei > > Either 'once' option or 'order' option can take effect for -boot at > the same time, that is say initial startup processing can check only > one. And pc.c's set_boot_dev() fails when its boot order argument > is invalid. This patch provide a solution fix this problem: > > 1. If "once" is given, register reset handler to restore boot order. > > 2. Pass the normal boot order to machine creation. Should fail when > the normal boot order is invalid. > > 3. If "once" is given, set it with qemu_boot_set(). Fails when the > once boot order is invalid. > > 4. Start the machine. > > 5. On reset, the reset handler calls qemu_boot_set() to restore boot > order. Should never fail. > > Suggested-by: Markus Armbruster > Signed-off-by: Gonglei > --- > vl.c | 23 +++++++++++++++-------- > 1 file changed, 15 insertions(+), 8 deletions(-) > > diff --git a/vl.c b/vl.c > index 983259b..24b4c38 100644 > --- a/vl.c > +++ b/vl.c > @@ -2734,6 +2734,7 @@ int main(int argc, char **argv, char **envp) > const char *initrd_filename; > const char *kernel_filename, *kernel_cmdline; > const char *boot_order; > + const char *boot_once = NULL; > DisplayState *ds; > int cyls, heads, secs, translation; > QemuOpts *hda_opts = NULL, *opts, *machine_opts, *icount_opts = NULL; > @@ -4045,8 +4046,7 @@ int main(int argc, char **argv, char **envp) > 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; > + const char *order; > Error *local_err = NULL; > > order = qemu_opt_get(opts, "order"); > @@ -4059,16 +4059,13 @@ int main(int argc, char **argv, char **envp) if (order) { validate_bootdevices(order, &local_err); if (local_err) { error_report_err(local_err); exit(1); } > boot_order = order; > } > > - once = qemu_opt_get(opts, "once"); > - if (once) { > - validate_bootdevices(once, &local_err); > + boot_once = qemu_opt_get(opts, "once"); > + if (boot_once) { > + validate_bootdevices(boot_once, &local_err); > if (local_err) { > error_report("%s", error_get_pretty(local_err)); > exit(1); > } > - 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(opts, "menu", boot_menu); Should work fine. There's a slight asymmetry, though: parameter "order" is fetched into a local variable, checked, and only then stored into its global variable. Parameter "once" goes straight to the global variable. Matter of taste, but treating them the same would be nice. Your choice. > @@ -4246,6 +4243,16 @@ int main(int argc, char **argv, char **envp) > > net_check_clients(); > > + if (boot_once) { > + Error *local_err = NULL; > + qemu_boot_set(boot_once, &local_err); > + if (local_err) { > + error_report("%s", error_get_pretty(local_err)); > + exit(1); > + } > + qemu_register_reset(restore_boot_order, g_strdup(boot_order)); > + } > + > ds = init_displaystate(); > > /* init local displays */