Re: [Xen-devel] [PATCH v2 16/18] libxl: introduce libxl_userdata_unlink

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Thu, Aug 28, 2014 at 08:31:39PM +0100, Ian Campbell wrote:
> On Thu, 2014-08-28 at 20:04 +0100, Wei Liu wrote:
> > Application locking is one thing, but we still need to serialise libxl
> > access to those files, don't we? Any access to userdata store via libxl
> > API should be serialised. The reason is stated in previous patch "libxl:
> > properly lock user data store".
> 
> I may be confused here, so please correct me if I'm wrong...
> 
> Any individual userdata store/retrieve is atomic insofar as afterwards
> there will be a consistent copy of the thing there, i.e. if there is a
> race you will get one or the other copy of the data, but never a
> mixture. Locking within the store/retrieve function neither helps nor
> hinders  this (since to the loser of the race the result is
> indistinguishable from someone coming along 1s later and updating).
> 
> The locking is there to protect against read-modify-write cycles (e.g.
> updating the config), which necessarily implies taking the lock before
> the read and releasing it after the write -- i.e. at the application
> layer (the libxl-lock being a kind of special in-libxl "application"
> layer). Without the lock two entities racing in the r-m-w cycle can
> result in updates being lost.

You're right on the r-m-w analysis. But the lock does more than that. In
this specific API family that manipulates userdata store, it also
ensures files won't disappear until other thread that holds the lock
finishes its job. Userdata vanishes under our feet is one abnormal state
we would like to avoid, userdata reappears after we delete it is another
abnormal state we would like to avoid.  If we don't hold this lock for
this unlink API, we now have the chance to come into those two abnormal
states. Does this make sense?

> > 
> > > > During review last round we discussed how we should deal with "xl
> > > > config-udpate" command. The conclusion is that we still honour user
> > > > supplied config file and it has higher priority than libxl-json. We
> > > > would like to transform the config file supplied by user to libxl-json,
> > > > then remove that user supplied file, so that next time domain is
> > > > rebooted it always has the config tracked by libxl. Without this patch
> > > > xl has no way to unlink that file and it will still take effect during
> > > > next reboot, which is not what we want.
> > > 
> > > OK, so this is about removing the existing xl config, not the
> > > libxl-json. I don't think this should take the libxl lock then -- that
> > > lock doesn't protect the xl cfg userdata in any meaningful way AFAICT.
> > > 
> > 
> > libxl-lock won't lock that file on application level but it is used to
> > prevent libxl threads from messing things up.
> > 
> > Does my explanation make sense? And, what do you have in mind for
> > designing this API?
> 
> I don't think we need or want this API, is my point, the libxl-lock
> should be used inside libxl to protect its updates of the cfg data, it
> should not be part of the core userdata primitives.
> 

OK, TBH I don't quite like this API either. If we don't provide a way
for xl to delete xl cfg userdata, what should we do with xl cfg? What do
you suggest to achieve the said behavior of "xl config-update"?

Wei.

> Perhaps Ian J disagrees though, or maybe I am confused.
> 
> Ian.

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