Re: [Xen-devel] [PATCH v1 03/10] libxl: store a copy of vanilla domain configuration when creating domain

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2014-07-16 at 17:47 +0100, Wei Liu wrote:
> On Wed, Jul 16, 2014 at 05:18:08PM +0100, Ian Campbell wrote:
> > On Thu, 2014-07-10 at 15:32 +0100, Wei Liu wrote:
> > 
> > > +/* update the saved domain configuration with a callback function,
> > > + * which is responsible to pull in useful fields from src.
> > > + */
> > > +typedef void (update_callback)(libxl__gc *, libxl_domain_config *,
> > > +                               const libxl_domain_config *);
> > > +static int libxl__update_domain_configuration(libxl__gc *gc,
> > > +                                              uint32_t domid,
> > > +                                              update_callback callback,
> > > +                                              const libxl_domain_config 
> > > *src)
> > 
> > You aren't using your shiny new locking functions here?
> > 
> 
> We don't need locking here. The lock should already be held (if it needs
> to be held).

OK. It's worth documenting that I think.

> Note, this is not async callback, this is just to make it possible to
> replace the update function at will. Probably renaming "update_callback"
> to "update" is better?

Either works for me.

> And for larger scope, when creating a domain, there shouldn't be any
> other thread trying to access the configuration file (is there?), so
> locking is not necessary.

Could someone race and try and call xl mem-set in the middle of the
create? Not sure. 

Always taking the lock when manipulating a domain's stored config, even
if it isn't strictly necessary means we don't need to think to hard
about those corner cases...

I suppose there is also a brief interval after the createdomain domctl
where the domid is "valid" but no config has been stored yet (since the
create doesn't hold the lock, I think). You might need some robustness
against that too.

Ian.


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