On Fri, May 21, 2021 at 06:07:35PM +0200, Greg Kurz wrote:
> QEMU 6.0 moved all the -boot variables to the machine. Especially, the
> removal of the boot_order static changed the handling of '-boot once'
> from:
>
> if (boot_once) {
> qemu_boot_set(boot_once, &error_fatal);
> qemu_register_reset(restore_boot_order, g_strdup(boot_order));
> }
>
> to
>
> if (current_machine->boot_once) {
> qemu_boot_set(current_machine->boot_once, &error_fatal);
> qemu_register_reset(restore_boot_order,
> g_strdup(current_machine->boot_order));
> }
>
> This means that we now register as subsequent boot order a copy
> of current_machine->boot_once that was just set with the previous
> call to qemu_boot_set(), i.e. we never transition away from the
> once boot order.
>
> It is certainly fragile^Wwrong for the spapr code to hijack a
> field of the base machine type object like that. The boot order
> rework simply turned this software boundary violation into an
> actual bug.
>
> Have the spapr code to handle that with its own field in
> SpaprMachineState. Also kfree() the initial boot device
> string when "once" was used.
>
> Fixes: 4b7acd2ac821 ("vl: clean up -boot variables")
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1960119
> Cc: pbonz...@redhat.com
> Signed-off-by: Greg Kurz <gr...@kaod.org>
Applied to ppc-for-6.1, thanks.
> ---
> include/hw/ppc/spapr.h | 3 +++
> hw/ppc/spapr.c | 8 +++++---
> 2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index bbf817af4647..f05219f75ef6 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -223,6 +223,9 @@ struct SpaprMachineState {
> int fwnmi_machine_check_interlock;
> QemuCond fwnmi_machine_check_interlock_cond;
>
> + /* Set by -boot */
> + char *boot_device;
> +
> /*< public >*/
> char *kvm_type;
> char *host_model;
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index c23bcc449071..4dd90b75cc52 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1005,7 +1005,7 @@ static void spapr_dt_chosen(SpaprMachineState *spapr,
> void *fdt, bool reset)
> _FDT(chosen = fdt_add_subnode(fdt, 0, "chosen"));
>
> if (reset) {
> - const char *boot_device = machine->boot_order;
> + const char *boot_device = spapr->boot_device;
> char *stdout_path = spapr_vio_stdout_path(spapr->vio_bus);
> size_t cb = 0;
> char *bootlist = get_boot_devices_list(&cb);
> @@ -2376,8 +2376,10 @@ static SaveVMHandlers savevm_htab_handlers = {
> static void spapr_boot_set(void *opaque, const char *boot_device,
> Error **errp)
> {
> - MachineState *machine = MACHINE(opaque);
> - machine->boot_order = g_strdup(boot_device);
> + SpaprMachineState *spapr = SPAPR_MACHINE(opaque);
> +
> + g_free(spapr->boot_device);
> + spapr->boot_device = g_strdup(boot_device);
> }
>
> static void spapr_create_lmb_dr_connectors(SpaprMachineState *spapr)
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
