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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2014-05-20 at 16:21 +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@xxxxxxxxxx>
> ...
> > @@ -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.
> 
> 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.

The device cases in particular are a lot harder to handle in that way.
This was how I originally envisaged this would work, Wei's approach
seemed to me to be much much simpler.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel