Re: [Qemu-devel] [PATCH 3/4] migration: spapr: migrate ccs_list in spapr state

[Michael Roth]

Quoting Daniel Henrique Barboza (2017-04-24 17:08:27)
> From: Jianjun Duan <address@hidden>
> 
> ccs_list in spapr state maintains the device tree related
> information on the rtas side for hotplugged devices. In racing
> situations between hotplug events and migration operation, a rtas
> hotplug event could be migrated from the source guest to target
> guest, or the source guest could have not yet finished fetching
> the device tree when migration is started, the target will try
> to finish fetching the device tree. By migrating ccs_list, the
> target can fetch the device tree properly.
> 
> In theory there would be other alternatives besides migrating the
> css_list to fix this. For example, we could add a flag that indicates
> whether a device is in the middle of the configure_connector during the
> migration process, in the post_load we can detect if this flag
> is active and then return an error informing the guest to restart the
> hotplug process. However, the DRC state can still be modified outside of
> hotplug. Using:
> 
>    drmgr -c pci -s <drc_index> -r
>    drmgr -c pci -s <drc_index> -a
> 
> it is possible to return a device to firmware and then later take it
> back and reconfigure it. This is not a common case but it's not prohibited,
> and performing a migration between these 2 operations would fail because
> the default coldplug state on target assumes a configured state in
> the source*. Migrating ccs_list is one solution that cover this
> case as well.
> 
> ccs_list is put in a subsection in the spapr state VMSD to make
> sure migration across different versions is not broken.
> 
> * see http://lists.nongnu.org/archive/html/qemu-devel/2016-10/msg01763.html
> for more information on this discussion.
> 
> Signed-off-by: Jianjun Duan <address@hidden>
> Signed-off-by: Daniel Henrique Barboza <address@hidden>
> ---
>  hw/ppc/spapr.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 35db949..22f351c 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1388,6 +1388,37 @@ static bool version_before_3(void *opaque, int 
> version_id)
>      return version_id < 3;
>  }
> 
> +static bool spapr_ccs_list_needed(void *opaque)
> +{
> +    sPAPRMachineState *spapr = (sPAPRMachineState *)opaque;
> +    return !QTAILQ_EMPTY(&spapr->ccs_list);
> +}
> +
> +static const VMStateDescription vmstate_spapr_ccs = {
> +    .name = "spaprconfigureconnectorstate",

some word separators wouldn't hurt, since they might show up in
migration error messages.

> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT32(drc_index, sPAPRConfigureConnectorState),
> +        VMSTATE_INT32(fdt_offset, sPAPRConfigureConnectorState),
> +        VMSTATE_INT32(fdt_depth, sPAPRConfigureConnectorState),
> +        VMSTATE_END_OF_LIST()
> +    },
> +};
> +
> +static const VMStateDescription vmstate_spapr_ccs_list = {
> +    .name = "spaprccslist",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .needed = spapr_ccs_list_needed,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_QTAILQ_V(ccs_list, sPAPRMachineState, 1,
> +                         vmstate_spapr_ccs, sPAPRConfigureConnectorState,
> +                         next),
> +        VMSTATE_END_OF_LIST()
> +    },
> +};
> +
>  static bool spapr_ov5_cas_needed(void *opaque)
>  {
>      sPAPRMachineState *spapr = opaque;
> @@ -1486,6 +1517,7 @@ static const VMStateDescription vmstate_spapr = {
>      .subsections = (const VMStateDescription*[]) {
>          &vmstate_spapr_ov5_cas,
>          &vmstate_spapr_patb_entry,
> +        &vmstate_spapr_ccs_list,
>          NULL
>      }
>  };
> -- 
> 2.9.3
> 
>