On Fri, Jan 30, 2015 at 12:00 AM, Gonglei <arei.gong...@huawei.com> wrote:
> On 2015/1/29 20:55, Alexander Graf wrote:
>
> >
> >
> > 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.
> >
>
> Just like what we said in other thread, remove the check about handler,
> I will post a patch soon.
>
Have you posted the patch? Cannot find it in the lists so I assume you have
not :-) Thanks!
>
> Regards,
> -Gonglei
>
>
>
--
--
Alexey
