Re: [Qemu-devel] [PATCH 2/2] hw/ppc/spapr Add qemu_register_boot_set for SPAPR

[Gonglei]

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, address@hidden wrote:
>>>> From: Dinar Valeev <address@hidden>
>>>>
>>>> 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 <address@hidden>
>>>> ---
>>>>   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.

Regards,
-Gonglei

> 
> diff --git a/bootdevice.c b/bootdevice.c
> index 5914417..3b750ff 100644
> --- a/bootdevice.c
> +++ b/bootdevice.c
> @@ -50,6 +50,7 @@ void qemu_register_boot_set(QEMUBootSetHandler *func, void 
> *opaque)
>  void qemu_boot_set(const char *boot_order, Error **errp)
>  {
>      Error *local_err = NULL;
> +    MachineState *machine = MACHINE(qdev_get_machine());
> 
>      if (!boot_set_handler) {
>          error_setg(errp, "no function defined to set boot device list for"
> @@ -63,6 +64,7 @@ void qemu_boot_set(const char *boot_order, Error **errp)
>          return;
>      }
> 
> +    machine->boot_order = boot_order;
>      boot_set_handler(boot_set_opaque, boot_order, errp);
>  }
> 
> 
> 
> Thanks,
> 
> Alex
> 
>