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

[Gerd Hoffmann]

  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.

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.

cheers,
  Gerd