On 29/01/18 10:43, Suraj Jitindar Singh wrote: > On Fri, 2018-01-19 at 16:18 +1100, Alexey Kardashevskiy wrote: >> On 19/01/18 16:00, Suraj Jitindar Singh wrote: >>> The vmstate description and the contained needed function for >>> migration >>> of spapr_caps is the same for each cap, with the name of the cap >>> substituted. As such introduce a macro to allow for easier >>> generation of >>> these. >>> >>> Convert the three existing spapr_caps (htm, vsx, and dfp) to use >>> this >>> macro. >>> >>> Signed-off-by: Suraj Jitindar Singh <sjitindarsi...@gmail.com> >>> >>> --- >>> >>> V5: >>> - Patch added to series >>> --- >>> hw/ppc/spapr_caps.c | 78 +++++++++++++++++---------------------- >>> -------------- >>> 1 file changed, 24 insertions(+), 54 deletions(-) >>> >>> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c >>> index d5c9ce774a..5d52969bd5 100644 >>> --- a/hw/ppc/spapr_caps.c >>> +++ b/hw/ppc/spapr_caps.c >>> @@ -228,62 +228,32 @@ int >>> spapr_caps_post_migration(sPAPRMachineState *spapr) >>> return ok ? 0 : -EINVAL; >>> } >>> >>> -static bool spapr_cap_htm_needed(void *opaque) >>> -{ >>> - sPAPRMachineState *spapr = opaque; >>> - >>> - return spapr->cmd_line_caps[SPAPR_CAP_HTM] && >>> - (spapr->eff.caps[SPAPR_CAP_HTM] != spapr- >>>> def.caps[SPAPR_CAP_HTM]); >>> -} >>> - >>> -const VMStateDescription vmstate_spapr_cap_htm = { >>> - .name = "spapr/cap/htm", >>> - .version_id = 1, >>> - .minimum_version_id = 1, >>> - .needed = spapr_cap_htm_needed, >>> - .fields = (VMStateField[]) { >>> - VMSTATE_UINT8(mig.caps[SPAPR_CAP_HTM], sPAPRMachineState), >>> - VMSTATE_END_OF_LIST() >>> - }, >>> -}; >>> - >>> -static bool spapr_cap_vsx_needed(void *opaque) >>> -{ >>> - sPAPRMachineState *spapr = opaque; >>> - >>> - return spapr->cmd_line_caps[SPAPR_CAP_VSX] && >>> - (spapr->eff.caps[SPAPR_CAP_VSX] != spapr- >>>> def.caps[SPAPR_CAP_VSX]); >>> +/* Used to generate the migration field and needed function for a >>> spapr cap */ >>> +#define SPAPR_CAP_MIG_STATE(cap, ccap) \ >>> +static bool spapr_cap_##cap##_needed(void *opaque) \ >>> +{ \ >>> + sPAPRMachineState *spapr = opaque; \ >>> + \ >>> + return spapr->cmd_line_caps[SPAPR_CAP_##ccap] && \ >>> + (spapr->eff.caps[SPAPR_CAP_##ccap] != \ >>> + spapr->def.caps[SPAPR_CAP_##ccap]); \ >>> +} \ >>> + \ >>> +const VMStateDescription vmstate_spapr_cap_##cap = { \ >>> + .name = "spapr/cap/" #cap, \ >>> + .version_id = 1, \ >>> + .minimum_version_id = 1, \ >>> + .needed = spapr_cap_##cap##_needed, \ >>> + .fields = (VMStateField[]) { \ >>> + VMSTATE_UINT8(mig.caps[SPAPR_CAP_##ccap], \ >>> + sPAPRMachineState), \ >>> + VMSTATE_END_OF_LIST() \ >>> + }, \ >>> } >>> >>> -const VMStateDescription vmstate_spapr_cap_vsx = { >>> - .name = "spapr/cap/vsx", >>> - .version_id = 1, >>> - .minimum_version_id = 1, >>> - .needed = spapr_cap_vsx_needed, >>> - .fields = (VMStateField[]) { >>> - VMSTATE_UINT8(mig.caps[SPAPR_CAP_VSX], sPAPRMachineState), >>> - VMSTATE_END_OF_LIST() >>> - }, >>> -}; >>> - >>> -static bool spapr_cap_dfp_needed(void *opaque) >>> -{ >>> - sPAPRMachineState *spapr = opaque; >>> - >>> - return spapr->cmd_line_caps[SPAPR_CAP_DFP] && >>> - (spapr->eff.caps[SPAPR_CAP_DFP] != spapr- >>>> def.caps[SPAPR_CAP_DFP]); >>> -} >>> - >>> -const VMStateDescription vmstate_spapr_cap_dfp = { >>> - .name = "spapr/cap/dfp", >>> - .version_id = 1, >>> - .minimum_version_id = 1, >>> - .needed = spapr_cap_dfp_needed, >>> - .fields = (VMStateField[]) { >>> - VMSTATE_UINT8(mig.caps[SPAPR_CAP_DFP], sPAPRMachineState), >>> - VMSTATE_END_OF_LIST() >>> - }, >>> -}; >>> +SPAPR_CAP_MIG_STATE(htm, HTM); >>> +SPAPR_CAP_MIG_STATE(vsx, VSX); >>> +SPAPR_CAP_MIG_STATE(dfp, DFP); >> >> >> A nit: I really dislike this kind of parameters which you cannot >> really >> cscope/ctags for because they are joined to some other things. >> >> "no matches found for cscope query g HTM of HTM" says vim's cscope >> when I >> search for "HTM". > > Would something like: > SPAPR_CAP_MIG_STATE(htm, SPAPR_CAP_HTM); > SPAPR_CAP_MIG_STATE(htm, SPAPR_CAP_VSX); > SPAPR_CAP_MIG_STATE(htm, SPAPR_CAP_DFP); > > be better?
It would indeed. > I could send a follow up Too late, David already merged :( And this seems to bother only me anyway. > >> >> >>> >>> void spapr_caps_reset(sPAPRMachineState *spapr) >>> { >>> >> >> -- Alexey