On 29.01.15 04:46, Gonglei wrote:
> On 2015/1/29 8:42, Alexander Graf wrote:
>
>>
>>
>> On 29.01.15 01:40, Gonglei wrote:
>>> On 2015/1/26 17:35, Alexander Graf wrote:
>>>
>>>> On 01/26/2015 11:49 AM, Dinar Valeev wrote:
>>>>> On 01/24/2015 12:04 AM, Alexander Graf wrote:
>>>>>>
>>>>>>
>>>>>> On 23.01.15 23:51, dval...@suse.de wrote:
>>>>>>> From: Dinar Valeev <dval...@suse.com>
>>>>>>>
>>>>>>> In order to have -boot once=d functioning, it is required to have
>>>>>>> qemu_register_boot_set
>>>>>>>
>>>>>>> qemu-system-ppc64 -enable-kvm -boot once=d
>>>>>>>
>>>>>>> Ready!
>>>>>>> 0 > dev /chosen ok
>>>>>>> 0 > .properties
>>>>>>> ...
>>>>>>> qemu,boot-device d
>>>>>>> ...
>>>>>>> 0 > reset-all
>>>>>>>
>>>>>>> Ready!
>>>>>>> 0 > dev /chosen ok
>>>>>>> 0 > .properties
>>>>>>> ...
>>>>>>> qemu,boot-device cdn
>>>>>>> ...
>>>>>>>
>>>>>>> Signed-off-by: Dinar Valeev <dval...@suse.com>
>>>>>>> ---
>>>>>>> hw/ppc/spapr.c | 12 ++++++++++++
>>>>>>> 1 file changed, 12 insertions(+)
>>>>>>>
>>>>>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>>>>>> index 3d2cfa3..38b03fc 100644
>>>>>>> --- a/hw/ppc/spapr.c
>>>>>>> +++ b/hw/ppc/spapr.c
>>>>>>> @@ -314,6 +314,16 @@ static void add_str(GString *s, const gchar *s1)
>>>>>>> g_string_append_len(s, s1, strlen(s1) + 1);
>>>>>>> }
>>>>>>>
>>>>>>> +static void spapr_boot_set(void *opaque, const char *boot_device,
>>>>>>> + Error **errp)
>>>>>>> +{
>>>>>>> + int offset;
>>>>>>> + offset = fdt_path_offset(opaque, "/chosen");
>>>>>>> + fdt_setprop_string(opaque, offset, "qemu,boot-device",
>>>>>>> boot_device);
>>>>>>> +
>>>>>>> +}
>>>>>>> +
>>>>>>> +
>>>>>>> static void *spapr_create_fdt_skel(hwaddr initrd_base,
>>>>>>> hwaddr initrd_size,
>>>>>>> hwaddr kernel_size,
>>>>>>> @@ -414,6 +424,8 @@ static void *spapr_create_fdt_skel(hwaddr
>>>>>>> initrd_base,
>>>>>>> if (boot_device) {
>>>>>>> _FDT((fdt_property_string(fdt, "qemu,boot-device",
>>>>>>> boot_device)));
>>>>>>> }
>>>>>>> + qemu_register_boot_set(spapr_boot_set, fdt);
>>>>>>
>>>>>> If you simply move the code above (the _FDT() one) from create_fdt_skel
>>>>>> to spapr_finalize_fdt() you should have the same net effect and much
>>>>>> cleaner code :).
>>>>> I've tried your proposal, on reset boot-device property stays "d"
>>>>
>>>> Ugh, the machine field doesn't change on reset. I think it'd be a lot more
>>>> intuitive if it did. Can you try with the patch below applied as well?
>>>>
>>>
>>> This approach is not good because boot_set_handler is NULL and return error
>>> directly.
>>> Please using qemu_register_boot_set for this purpose.
>>
>> I'd personally prefer if we get rid of qemu_register_boot_set
>> completely. It duplicates the reset logic as well as information holder
>> locality (machine struct vs parameter).
>>
>
> Maybe yes. But lots of other machines do not register
> reset callback. So those machines using qemu_register_boot_set()
> register a handler callback achieve this purpose.
I think we're better off just registering reset handlers then.
Alex
