From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH v2 11/18] libxl: synchronise configuration when we hotplug a device Date: Wed, 3 Sep 2014 12:57:09 +0100 Message-ID: <1409745429.20794.8.camel@citrix.com> References: <1406744639-28782-1-git-send-email-wei.liu2@citrix.com> <1406744639-28782-12-git-send-email-wei.liu2@citrix.com> <1409104839.28009.73.camel@citrix.com> <20140828111828.GJ13165@zion.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20140828111828.GJ13165@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:18 +0100, Wei Liu wrote: > On Wed, Aug 27, 2014 at 03:00:39AM +0100, Ian Campbell wrote: > > On Wed, 2014-07-30 at 19:23 +0100, Wei Liu wrote: > > > As those routines are called both during domain creation and device > > > hotplug, we add a flag to indicate whether we need to update JSON > > > config. This flag is only set to true when we hotplug a device. We > > > cannot update JSON config during domain creation as JSON config is > > > committed to disk only when domain creation finishes. > > > > Rather than carry a flag around did you consider just checking for the > > presence of the file? I think you indicated in an earlier patch that you > > were going to treat lack of the file as meaning creation/destruction was > > happening. > > > > I think a flag is more explict. > > These libxl__device_*_add functions are called under two circumstances, > a) domain creation, b) device hotplug. > > In b) if the file is not present it could also be in abnormal state -- > file is deleted by accident, file system error etc. Carrying a flag can > help up distinguish normal and abnormal state. (Though I can see my code > currently lacks checking this at the moment) OK. > > > > > + * They take 6 parameters: > > > + * type: the type of the device, say nic, vtpm, disk, pci etc > > > + * ptr: the pointer to array inside libxl_domain_config > > > > To the array or to a specific element of the array? I think the latter. > > To the array of libxl_device_#type. It doesn't point to specific > element. OK. "...pointer to the start of the array..." would be unambiguous (assuming the later comment about pasting num_ prefixes didn't make this comment go away altogether) Ian.