On Thu, Jun 08, 2017 at 07:27:42PM +0200, Laurent Vivier wrote: > If the OS is not started, QEMU sends an event to the OS > that is lost and cannot be recovered. An unplug is not > able to restore QEMU in a coherent state. > So, while the OS is not started, disable CPU and memory hotplug. > We guess the OS is started if the CAS has been negotiated. > > Signed-off-by: Laurent Vivier <lviv...@redhat.com>
It seems a pain to introduce a whole new (migrated) variable just to
check this. Could we instead tweak the allocation of spapr->ov5_cas,
so it is NULL until CAS is completed?
> ---
> hw/ppc/spapr.c | 51
> +++++++++++++++++++++++++++++++++++++++++++++++---
> hw/ppc/spapr_hcall.c | 1 +
> include/hw/ppc/spapr.h | 2 ++
> 3 files changed, 51 insertions(+), 3 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 91b4057..4c979d5 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1308,6 +1308,7 @@ static void ppc_spapr_reset(void)
> {
> MachineState *machine = MACHINE(qdev_get_machine());
> sPAPRMachineState *spapr = SPAPR_MACHINE(machine);
> + sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(machine);
> PowerPCCPU *first_ppc_cpu;
> uint32_t rtas_limit;
> hwaddr rtas_addr, fdt_addr;
> @@ -1373,6 +1374,7 @@ static void ppc_spapr_reset(void)
> first_ppc_cpu->env.nip = SPAPR_ENTRY_POINT;
>
> spapr->cas_reboot = false;
> + spapr->cas_completed = smc->cas_completed_default;
I also dislike the cas_completed_default thing. If
cas_completed_default != false, it means that we will have
cas_completed == true when we have not, in fact, completed CAS. I see
why you're doing this for migration compat, but that seems like a
recipe for confusion in the future.
> }
>
> static void spapr_create_nvram(sPAPRMachineState *spapr)
> @@ -1528,6 +1530,27 @@ static const VMStateDescription
> vmstate_spapr_patb_entry = {
> },
> };
>
> +static bool spapr_cas_completed_needed(void *opaque)
> +{
> + sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(opaque);
> +
> + /* we need to migrate cas_completed only if it is
> + * not set by default
> + */
> + return !smc->cas_completed_default;
> +}
> +
> +static const VMStateDescription vmstate_spapr_cas_completed = {
> + .name = "spapr_cas_completed",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .needed = spapr_cas_completed_needed,
> + .fields = (VMStateField[]) {
> + VMSTATE_BOOL(cas_completed, sPAPRMachineState),
> + VMSTATE_END_OF_LIST()
> + },
> +};
> +
> static const VMStateDescription vmstate_spapr = {
> .name = "spapr",
> .version_id = 3,
> @@ -1546,6 +1569,7 @@ static const VMStateDescription vmstate_spapr = {
> .subsections = (const VMStateDescription*[]) {
> &vmstate_spapr_ov5_cas,
> &vmstate_spapr_patb_entry,
> + &vmstate_spapr_cas_completed,
> NULL
> }
> };
> @@ -2599,6 +2623,7 @@ out:
> static void spapr_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState
> *dev,
> Error **errp)
> {
> + sPAPRMachineState *ms = SPAPR_MACHINE(hotplug_dev);
> PCDIMMDevice *dimm = PC_DIMM(dev);
> PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
> MemoryRegion *mr = ddc->get_memory_region(dimm);
> @@ -2617,6 +2642,14 @@ static void spapr_memory_pre_plug(HotplugHandler
> *hotplug_dev, DeviceState *dev,
> "Use 'memory-backend-file' with correct mem-path.");
> return;
> }
> + if (dev->hotplugged) {
> + if (!runstate_check(RUN_STATE_PRELAUNCH) &&
> + !runstate_check(RUN_STATE_INMIGRATE) &&
> + !ms->cas_completed) {
> + error_setg(errp, "Memory hotplug not supported without OS");
> + return;
> + }
> + }
> }
>
> struct sPAPRDIMMState {
> @@ -2915,6 +2948,7 @@ static void spapr_core_pre_plug(HotplugHandler
> *hotplug_dev, DeviceState *dev,
> Error **errp)
> {
> MachineState *machine = MACHINE(OBJECT(hotplug_dev));
> + sPAPRMachineState *ms = SPAPR_MACHINE(machine);
> MachineClass *mc = MACHINE_GET_CLASS(hotplug_dev);
> Error *local_err = NULL;
> CPUCore *cc = CPU_CORE(dev);
> @@ -2923,9 +2957,18 @@ static void spapr_core_pre_plug(HotplugHandler
> *hotplug_dev, DeviceState *dev,
> CPUArchId *core_slot;
> int index;
>
> - if (dev->hotplugged && !mc->has_hotpluggable_cpus) {
> - error_setg(&local_err, "CPU hotplug not supported for this machine");
> - goto out;
> + if (dev->hotplugged) {
> + if (!mc->has_hotpluggable_cpus) {
> + error_setg(&local_err,
> + "CPU hotplug not supported for this machine");
> + goto out;
> + }
> + if (!runstate_check(RUN_STATE_PRELAUNCH) &&
> + !runstate_check(RUN_STATE_INMIGRATE) &&
> + !ms->cas_completed) {
> + error_setg(&local_err, "CPU hotplug not supported without OS");
> + goto out;
> + }
> }
>
> if (strcmp(base_core_type, type)) {
> @@ -3358,9 +3401,11 @@ static void
> spapr_machine_2_9_instance_options(MachineState *machine)
>
> static void spapr_machine_2_9_class_options(MachineClass *mc)
> {
> + sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
> spapr_machine_2_10_class_options(mc);
> SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_9);
> mc->numa_auto_assign_ram = numa_legacy_auto_assign_ram;
> + smc->cas_completed_default = true;
> }
>
> DEFINE_SPAPR_MACHINE(2_9, "2.9", false);
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index aa1ffea..d561a00 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1185,6 +1185,7 @@ static target_ulong
> h_client_architecture_support(PowerPCCPU *cpu,
> }
> }
>
> + spapr->cas_completed = true;
> return H_SUCCESS;
> }
>
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index f973b02..f5835db 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -57,6 +57,7 @@ struct sPAPRMachineClass {
> uint64_t *buid, hwaddr *pio,
> hwaddr *mmio32, hwaddr *mmio64,
> unsigned n_dma, uint32_t *liobns, Error **errp);
> + bool cas_completed_default;
> };
>
> /**
> @@ -90,6 +91,7 @@ struct sPAPRMachineState {
> sPAPROptionVector *ov5_cas; /* negotiated (via CAS) option vectors */
> bool cas_reboot;
> bool cas_legacy_guest_workaround;
> + bool cas_completed;
>
> Notifier epow_notifier;
> QTAILQ_HEAD(, sPAPREventLogEntry) pending_events;
--
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
