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

[Alexander Graf]

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?


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