From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51707) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YM5wD-0006XL-41 for qemu-devel@nongnu.org; Thu, 12 Feb 2015 21:25:02 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YM5w8-0005kk-5T for qemu-devel@nongnu.org; Thu, 12 Feb 2015 21:25:01 -0500 Received: from szxga02-in.huawei.com ([119.145.14.65]:22276) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YM5w7-0005kI-Is for qemu-devel@nongnu.org; Thu, 12 Feb 2015 21:24:56 -0500 Message-ID: <54DD6060.3050801@huawei.com> Date: Fri, 13 Feb 2015 10:24:32 +0800 From: Gonglei MIME-Version: 1.0 References: <1423293616-10736-1-git-send-email-arei.gonglei@huawei.com> <1423293616-10736-3-git-send-email-arei.gonglei@huawei.com> <87sieba0u5.fsf@blackfin.pond.sub.org> In-Reply-To: <87sieba0u5.fsf@blackfin.pond.sub.org> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit 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: Markus Armbruster Cc: peter.huangpeng@huawei.com, dvaleev@suse.de, qemu-devel@nongnu.org, agraf@suse.de On 2015/2/12 18:19, Markus Armbruster wrote: > 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. > Agree, I get your meaning. :) Regards, -Gonglei >> @@ -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 */