From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57702) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YJfBE-0005Wy-Sx for qemu-devel@nongnu.org; Fri, 06 Feb 2015 04:26:30 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YJfBA-00079x-PL for qemu-devel@nongnu.org; Fri, 06 Feb 2015 04:26:28 -0500 Received: from szxga01-in.huawei.com ([119.145.14.64]:45897) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YJfB9-00077y-Vp for qemu-devel@nongnu.org; Fri, 06 Feb 2015 04:26:24 -0500 Message-ID: <54D488AC.3050304@huawei.com> Date: Fri, 6 Feb 2015 17:26:04 +0800 From: Gonglei MIME-Version: 1.0 References: <1422966857-6776-1-git-send-email-arei.gonglei@huawei.com> <1422966857-6776-3-git-send-email-arei.gonglei@huawei.com> <87ioffwgs4.fsf@blackfin.pond.sub.org> In-Reply-To: <87ioffwgs4.fsf@blackfin.pond.sub.org> Content-Type: text/plain; charset="GB2312" Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 2/3] 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: "Huangpeng (Peter)" , "dvaleev@suse.de" , "qemu-devel@nongnu.org" , "agraf@suse.de" On 2015/2/6 17:06, 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 | 13 +++++++++++-- >> 1 file changed, 11 insertions(+), 2 deletions(-) >> >> diff --git a/vl.c b/vl.c >> index 983259b..0d90d98 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 *once = NULL; > > Consider renaming once to boot_once. In its much larger scope, the boot > context isn't obvious anymore, so a more telling name would be in order. > Agree. >> DisplayState *ds; >> int cyls, heads, secs, translation; >> QemuOpts *hda_opts = NULL, *opts, *machine_opts, *icount_opts = NULL; >> @@ -4046,7 +4047,7 @@ int main(int argc, char **argv, char **envp) >> 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"); >> @@ -4067,7 +4068,6 @@ int main(int argc, char **argv, char **envp) >> exit(1); >> } >> normal_boot_order = g_strdup(boot_order); >> - boot_order = once; >> qemu_register_reset(restore_boot_order, normal_boot_order); >> } >> > > normal_boot_order can be eliminated now. > > } > qemu_register_reset(restore_boot_order, strdup(order)); > } > > Even better, move qemu_register_reset()... > >> @@ -4246,6 +4246,15 @@ int main(int argc, char **argv, char **envp) >> >> net_check_clients(); >> >> + if (once) { >> + Error *local_err = NULL; >> + qemu_boot_set(once, &local_err); >> + if (local_err) { >> + error_report("%s", error_get_pretty(local_err)); >> + exit(1); >> + } > > ... here: > > qemu_register_reset(restore_boot_order, strdup(boot_order)); > >> + } >> + >> ds = init_displaystate(); >> >> /* init local displays */ > > Clearer, don't you think? Yes, it's cool. :) Regards, -Gonglei