Re: [PATCH v9 0/8] domain context infrastructure

[Tamas K Lengyel <tamas.k.lengyel@xxxxxxxxx>]

On Tue, Sep 29, 2020 at 7:54 AM Durrant, Paul <pdurrant@xxxxxxxxxxxx> wrote:
>
> > -----Original Message-----
> > From: Lengyel, Tamas <tamas.lengyel@xxxxxxxxx>
> > Sent: 28 September 2020 15:17
> > To: paul@xxxxxxx; xen-devel@xxxxxxxxxxxxxxxxxxxx
> > Cc: Durrant, Paul <pdurrant@xxxxxxxxxxxx>; 'Andrew Cooper' 
> > <andrew.cooper3@xxxxxxxxxx>; 'Daniel De
> > Graaf' <dgdegra@xxxxxxxxxxxxx>; 'George Dunlap' <george.dunlap@xxxxxxxxxx>; 
> > 'Ian Jackson'
> > <ian.jackson@xxxxxxxxxxxxx>; 'Jan Beulich' <jbeulich@xxxxxxxx>; 'Julien 
> > Grall' <julien@xxxxxxx>;
> > 'Marek Marczykowski-Górecki' <marmarek@xxxxxxxxxxxxxxxxxxxxxx>; 'Roger Pau 
> > Monné'
> > <roger.pau@xxxxxxxxxx>; 'Stefano Stabellini' <sstabellini@xxxxxxxxxx>; 
> > 'Volodymyr Babchuk'
> > <Volodymyr_Babchuk@xxxxxxxx>; 'Wei Liu' <wl@xxxxxxx>
> > Subject: RE: [EXTERNAL] [PATCH v9 0/8] domain context infrastructure
> >
> > CAUTION: This email originated from outside of the organization. Do not 
> > click links or open
> > attachments unless you can confirm the sender and know the content is safe.
> >
> >
> >
> > > > Hi Paul,
> > > > Could you push a git branch somewhere for this series? I would like to
> > > > see this being integrated with VM forking and if its not too much
> > > > effort just create the patch for that so that it could be appended to 
> > > > the
> > > series.
> > > >
> > >
> > > Hi Tamas,
> > >
> > >   Done. See
> > > https://xenbits.xen.org/gitweb/?p=people/pauldu/xen.git;a=shortlog;h=refs/h
> > > eads/domain-save14
> > >
> > >   Cheers,
> > >
> > >     Paul
> >
> > Hi Paul,
> > I added a small patch that would save & load the PV context from one domain 
> > to another that would be
> > called during VM forking. Please take a look at
> > https://xenbits.xen.org/gitweb/?p=people/tklengyel/xen.git;a=commitdiff;h=1843ca7302e415317fdb9a63b3a4
> > d29a385dc766;hp=8149296fdf80c73727e61cea6fe3251aecf8b333. I called the 
> > function copy_pv_domaincontext
> > for now as that seemed like the most appropriate description for it. Please 
> > let me know if this looks
> > good to you. I'm still testing it but if everything checks out it would be 
> > nice to just append this
> > patch to your series.
>
> Hi Tamas,
>
>   The code structure appears to be ok... just some cosmetic tweaks:
>
> - I think you should call the function simply 'copy_domaincontext' as the 
> idea is that all state (including what is now in hvm context) will be 
> consolidated

Sure, I wasn't entirely clear about whether this will be limited to PV
context or if it will eventually add the hvm stuff too. Right now I
still would have to do that separately.

> - The prevailing style in domctl.c AFAICS is that assignments are mostly not 
> done inside if statements. Personally I think this is a good thing.

I think it cuts down on function sizes when all that is being done
after an assigment is a NULL-check. No need for a separate line for it
but I also don't care that much. So if it's more important to whoever
maintains this to keep the style consistent in this regard I can
change it.

>
>   Once you have something ready to go then I'd be happy to tag it onto my 
> series if I need to do a v10... but I'm currently hoping that won't be 
> necessary.

I think I'll wait until HVM context is included in the framework as
well so that we can just switch over everything at once.

Tamas