[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 2/3] bootdevice: check boot order argument va
From: |
Gonglei |
Subject: |
Re: [Qemu-devel] [PATCH v2 2/3] bootdevice: check boot order argument validation before vm running |
Date: |
Fri, 6 Feb 2015 17:26:04 +0800 |
User-agent: |
Mozilla/5.0 (Windows NT 6.1; rv:11.0) Gecko/20120327 Thunderbird/11.0.1 |
On 2015/2/6 17:06, Markus Armbruster wrote:
> <address@hidden> writes:
>
>> From: Gonglei <address@hidden>
>>
>> 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 <address@hidden>
>> Signed-off-by: Gonglei <address@hidden>
>> ---
>> 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