Re: [Qemu-devel] [PATCH v6 07/27] vl.c: add setter/getter functions for bootindex property

[Gonglei (Arei)]

> From: Gerd Hoffmann [mailto:address@hidden
> 
>   Hi,
> 
> > +void set_bootindex(int32_t *bootindex, Visitor *v,
> > > +                   const char *name, Error **errp)
> > > +{
> > > +    int32_t boot_index;
> > > +    Error *local_err = NULL;
> > > +
> > > +    visit_type_int32(v, &boot_index, name, &local_err);
> > > +
> > > +    if (local_err == NULL) {
> > > +        /* check the bootindex existes or not in fw_boot_order list  */
> > > +        check_boot_index(boot_index, &local_err);
> > > +    }
> > > +
> > > +    if (local_err) {
> > > +        error_propagate(errp, local_err);
> > > +        return;
> > > +    }
> > > +    /* change bootindex to a new one */
> > > +    *bootindex = boot_index;
> > > +}
> > > +
> >
> > In this version, I just change devices' bootindex value, but not update
> fw_boot_order
> > immediately. When we reboot the vm, we will call add_boot_device_path to
> update
> > fw_boot_order list. Do you think it is a good method? This method will cause
> a problem
> > that when we want set a existed bootindex which has been changed, we will
> get failure.
> > Only after vm rebooting, we can set the same bootindex again.
> 
> Good point.  check_boot_index() doing the verification against stale
> data is pointless and may even throw an error in cases where it should
> not.
> 
Yes.

> I guess we should add a add_boot_device_path() call to the
> ${device}_set_bootindex functions.  Which also means we don't need to do
> it on reset (patch #6 can be dropped).  And I think we can also drop the
> add_boot_device_path() calls from ${device}_realize then.
> 
Bravo! I can't agree more with you. Thanks, Gerd.

Will do it.

Best regards,
-Gonglei