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

[Ian Campbell <ian.campbell@citrix.com>]

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.
> 
> > > 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.

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

Ian.