From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH v2 16/18] libxl: introduce libxl_userdata_unlink Date: Thu, 28 Aug 2014 19:20:31 +0100 Message-ID: <1409250031.21481.25.camel@citrix.com> References: <1406744639-28782-1-git-send-email-wei.liu2@citrix.com> <1406744639-28782-17-git-send-email-wei.liu2@citrix.com> <1409105819.28009.82.camel@citrix.com> <20140828115043.GN13165@zion.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20140828115043.GN13165@zion.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Wei Liu Cc: ian.jackson@eu.citrix.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On Thu, 2014-08-28 at 12:50 +0100, Wei Liu wrote: > On Wed, Aug 27, 2014 at 03:16:59AM +0100, Ian Campbell wrote: > > On Wed, 2014-07-30 at 19:23 +0100, Wei Liu wrote: > > > + CTX_LOCK; > > > + lock = libxl__lock_domain_data(gc, domid); > > > + if (!lock) { > > > > This seems like a quirk which deserves commenting on since this lock > > only protects a specific type of userdata, specifically the libxl-json > > userdata. > > > > The original intent for that lock is to protect all application defined > data -- the name in registry is "libxl-lock" not "libxl-json-lock". libxl-lock suggests to me that it locks things internal to libxl, not "application userdata lock". > Though through out this series it's mostly used to protect libxl-json > data, it doesn't preclude us from using it to protect other type of > userdata. > Except the locking functions aren't public, are they? and since the interface uses carefd's I don't think it can be (easily) made public. I guess what I'm saying is that if an app wants to lock accesses to its userdata then it has to do that itself, it can't be done internally to libxl. > 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. Ian.