Re: [PATCH V5 31/32] libxl: update domain configuration when updating memory targets

[Wei Liu <wei.liu2@citrix.com>]

On Tue, May 20, 2014 at 04:21:26PM +0100, Ian Jackson wrote:
> Wei Liu writes ("[PATCH V5 31/32] libxl: update domain configuration when updating memory targets"):
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ...
> > @@ -4005,6 +4024,14 @@ int libxl_domain_setmaxmem(libxl_ctx *ctx, uint32_t domid, uint32_t max_memkb)
> >          goto out;
> >      }
> >  
> > +    {
> > +        LOAD_DOMAIN_CONFIG(domid);
> > +
> > +        d_config.b_info.max_memkb = max_memkb;
> > +
> > +        STORE_DOMAIN_CONFIG(domid);
> 
> I think this is a fundamentally hazardous way to deal with domain
> configuration updates.
> 
> The result is that there are two places where the domain's maximum
> memory is recorded - the actual running domain state, and the saved
> json configuration.  In error situations it is possible for these to
> become out of step.
> 
> What you should do instead is have the domain configuration retrieval
> code (currently known as libxl_load_domain_configuration) do
> xc_domain_getmaxmem (or whatever it is) and update the value in the
> config data it is about to return.
> 

So you're actually suggesting I use the store JSON config as tempalte
and tempalte only, then pull in other runtime information in retrieval
code, right? This is not what I thought in the first place. I was
expecting libxl-json file always stores the current up to date
configuration.

I can see your approach can work better than mine. My only concern is
that some curious user might want to poke at libxl-json file, i.e. try
to get domain configuration without programming against libxl API. But I
think that should be forbidden, right?

Wei.

> That way there is only one place where the maxmem information is
> stored.  (There is of course another copy in the json config state but
> it will always be ignored so can be disregarded.)  So it is not
> possible for the system to be in an inconsistent state.
> 
> The same considerations apply to device addition and removal, vif MAC
> address update, etc.
> 
> Ian.