|
From: | Daniel Henrique Barboza |
Subject: | Re: [Qemu-ppc] [PATCH v9 4/6] hw/ppc/spapr.c: migrate pending_dimm_unplugs of spapr state |
Date: | Fri, 12 May 2017 16:54:57 -0300 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.0 |
On 05/12/2017 03:12 AM, David Gibson wrote:
On Fri, May 05, 2017 at 05:47:44PM -0300, Daniel Henrique Barboza wrote:To allow for a DIMM unplug event to resume its work if a migration occurs in the middle of it, this patch migrates the non-empty pending_dimm_unplugs QTAILQ that stores the DIMM information that the spapr_lmb_release() callback uses. It was considered an apprach where the DIMM states would be restored on the post-_load after a migration. The problem is that there is no way of knowing, from the sPAPRMachineState, if a given DIMM is going through an unplug process and the callback needs the updated DIMM State. We could migrate a flag indicating that there is an unplug event going on for a certain DIMM, fetching this information from the start of the spapr_del_lmbs call. But this would also require a scan on post_load to figure out how many nr_lmbs are left. At this point we can just migrate the nr_lmbs information as well, given that it is being calculated at spapr_del_lmbs already, and spare a scanning/discovery in the post-load. All that we need is inside the sPAPRDIMMState structure that is added to the pending_dimm_unplugs queue at the start of the spapr_del_lmbs, so it's convenient to just migrated this queue it if it's not empty. Signed-off-by: Daniel Henrique Barboza <address@hidden>NACK. As I believe I suggested previously, you can reconstruct this state on the receiving side by doing a full scan of the DIMM and LMB DRC states.
Just had an idea that I think it's in the line of what you're suggesting. Given
that the information we need is only created in the spapr_del_lmbs (as per patch 1), we can use the absence of this information in the release callback as a sort of a flag, an indication that a migration got in the way and we need to reconstruct the nr_lmbs states again, using the same scanning function I've used in v8. The flow would be like this (considering the changes in the previous 3 patches so far): ------------ /* Callback to be called during DRC release. */ void spapr_lmb_release(DeviceState *dev) { HotplugHandler *hotplug_ctrl; uint64_t addr = spapr_dimm_get_address(PC_DIMM(dev)); sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine()); sPAPRDIMMState *ds = spapr_pending_dimm_unplugs_find(spapr, addr);// no DIMM state found in spapr - re-create it to find out how may LMBs are left
if (ds == NULL) { uint32 nr_lmbs = ***call_scanning_LMB_DRCs_function(dev)*** // recreate the sPAPRDIMMState element and add it back to spapr } ( resume callback as usual ) ----------- Is this approach be adequate? Another alternative would be to use another way of detecting if an LMB unplug is happening and, if positive, do the sameprocess in the post_load(). In this case I'll need to take a look in the code and
see how we can detect an ongoing unplug besides what I've said above. Thanks, Daniel
--- hw/ppc/spapr.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index e190eb9..30f0b7b 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -1437,6 +1437,36 @@ static bool version_before_3(void *opaque, int version_id) return version_id < 3; }+static bool spapr_pending_dimm_unplugs_needed(void *opaque)+{ + sPAPRMachineState *spapr = (sPAPRMachineState *)opaque; + return !QTAILQ_EMPTY(&spapr->pending_dimm_unplugs); +} + +static const VMStateDescription vmstate_spapr_dimmstate = { + .name = "spapr_dimm_state", + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_UINT64(addr, sPAPRDIMMState), + VMSTATE_UINT32(nr_lmbs, sPAPRDIMMState), + VMSTATE_END_OF_LIST() + }, +}; + +static const VMStateDescription vmstate_spapr_pending_dimm_unplugs = { + .name = "spapr_pending_dimm_unplugs", + .version_id = 1, + .minimum_version_id = 1, + .needed = spapr_pending_dimm_unplugs_needed, + .fields = (VMStateField[]) { + VMSTATE_QTAILQ_V(pending_dimm_unplugs, sPAPRMachineState, 1, + vmstate_spapr_dimmstate, sPAPRDIMMState, + next), + VMSTATE_END_OF_LIST() + }, +}; + static bool spapr_ov5_cas_needed(void *opaque) { sPAPRMachineState *spapr = opaque; @@ -1535,6 +1565,7 @@ static const VMStateDescription vmstate_spapr = { .subsections = (const VMStateDescription*[]) { &vmstate_spapr_ov5_cas, &vmstate_spapr_patb_entry, + &vmstate_spapr_pending_dimm_unplugs, NULL } };
[Prev in Thread] | Current Thread | [Next in Thread] |