> -----Original Message----- > From: Jan Beulich <jbeul...@suse.com> > Sent: 19 May 2020 16:37 > To: p...@xen.org > Cc: xen-devel@lists.xenproject.org; 'Paul Durrant' <pdurr...@amazon.com>; > 'Andrew Cooper' > <andrew.coop...@citrix.com>; 'George Dunlap' <george.dun...@citrix.com>; 'Ian > Jackson' > <ian.jack...@eu.citrix.com>; 'Julien Grall' <jul...@xen.org>; 'Stefano > Stabellini' > <sstabell...@kernel.org>; 'Wei Liu' <w...@xen.org>; 'Volodymyr Babchuk' > <volodymyr_babc...@epam.com>; > 'Roger Pau Monné' <roger....@citrix.com> > Subject: Re: [PATCH v3 1/5] xen/common: introduce a new framework for > save/restore of 'domain' context > > On 19.05.2020 17:32, Paul Durrant wrote: > >> -----Original Message----- > >> From: Jan Beulich <jbeul...@suse.com> > >> Sent: 19 May 2020 16:18 > >> To: p...@xen.org > >> Cc: xen-devel@lists.xenproject.org; 'Paul Durrant' <pdurr...@amazon.com>; > >> 'Andrew Cooper' > >> <andrew.coop...@citrix.com>; 'George Dunlap' <george.dun...@citrix.com>; > >> 'Ian Jackson' > >> <ian.jack...@eu.citrix.com>; 'Julien Grall' <jul...@xen.org>; 'Stefano > >> Stabellini' > >> <sstabell...@kernel.org>; 'Wei Liu' <w...@xen.org>; 'Volodymyr Babchuk' > >> <volodymyr_babc...@epam.com>; > >> 'Roger Pau Monné' <roger....@citrix.com> > >> Subject: Re: [PATCH v3 1/5] xen/common: introduce a new framework for > >> save/restore of 'domain' > context > >> > >> On 19.05.2020 17:10, Paul Durrant wrote: > >>>> From: Jan Beulich <jbeul...@suse.com> > >>>> Sent: 19 May 2020 15:24 > >>>> > >>>> On 19.05.2020 16:04, Paul Durrant wrote: > >>>>>> From: Jan Beulich <jbeul...@suse.com> > >>>>>> Sent: 19 May 2020 14:04 > >>>>>> > >>>>>> On 14.05.2020 12:44, Paul Durrant wrote: > >>>>>>> +/* > >>>>>>> + * Register save and restore handlers. Save handlers will be invoked > >>>>>>> + * in order of DOMAIN_SAVE_CODE(). > >>>>>>> + */ > >>>>>>> +#define DOMAIN_REGISTER_SAVE_RESTORE(_x, _save, _load) \ > >>>>>>> + static int __init __domain_register_##_x##_save_restore(void) \ > >>>>>>> + { \ > >>>>>>> + domain_register_save_type( \ > >>>>>>> + DOMAIN_SAVE_CODE(_x), \ > >>>>>>> + #_x, \ > >>>>>>> + &(_save), \ > >>>>>>> + &(_load)); \ > >>>>>>> + \ > >>>>>>> + return 0; \ > >>>>>>> + } \ > >>>>>>> + __initcall(__domain_register_##_x##_save_restore); > >>>>>> > >>>>>> I'm puzzled by part of the comment: Invoking by save code looks > >>>>>> reasonable for the saving side (albeit END doesn't match this rule > >>>>>> afaics), but is this going to be good enough for the consuming side? > >>>>> > >>>>> No, this only relates to the save side which is why the comment > >>>>> says 'Save handlers'. I do note that it would be more consistent > >>>>> to use 'load' rather than 'restore' here though. > >>>>> > >>>>>> There may be dependencies between types, and with fixed ordering > >>>>>> there may be no way to insert a depended upon type ahead of an > >>>>>> already defined one (at least as long as the codes are meant to be > >>>>>> stable). > >>>>>> > >>>>> > >>>>> The ordering of load handlers is determined by the stream. I'll > >>>>> add a sentence saying that. > >>>> > >>>> I.e. the consumer of the "get" interface (and producer of the stream) > >>>> is supposed to take apart the output it gets, bring records into > >>>> suitable order (which implies it knows of all the records, and which > >>>> hence means this code may need updating in cases where I'd expect > >>>> only the hypervisor needs), and only then issue to the stream? > >>> > >>> The intention is that the stream is always in a suitable order so the > >>> load side does not have to do any re-ordering. > >> > >> I understood this to be the intention, but what I continue to not > >> understand is where / how the save side orders it suitably. "Save > >> handlers will be invoked in order of DOMAIN_SAVE_CODE()" does not > >> allow for any ordering, unless at the time of the introduction of > >> a particular code you already know what others it may depend on > >> in the future, reserving appropriate codes. > >> > > > > That's just how it is *now*. If a new code is defined that needs to > > be in the stream before one of the existing ones then we'll have to > > introduce a more elaborate scheme to deal with that at the time. > > Using the save code as the array index and iterating in that order > > is purely a convenience, and the load side does not depend on > > entries being in save code order. > > Could then you make the comment indicate so? This will allow people > wanting to modify this do so more easily, without much digging in > code or mail history.
Ok, I'll add some words to that effect. Paul > > Jan