From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH v2] xentop: add support for qdisks Date: Fri, 20 Mar 2015 09:56:18 +0000 Message-ID: <1426845378.21742.113.camel@citrix.com> References: <55002C5B02000091000F1DAD@prv-mh.provo.novell.com> <1426695146.14291.60.camel@citrix.com> <550AB7F702000091000F2C6D@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <550AB7F702000091000F2C6D@prv-mh.provo.novell.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: Charles Arnold Cc: Anthony Perard , xen-devel List-Id: xen-devel@lists.xenproject.org On Thu, 2015-03-19 at 11:50 -0600, Charles Arnold wrote: > > Whether the interface exists (even in buggy form) or not in older > > versions is important because if it doesn't exist then that would be a > > build failure, which we would want to avoid. > > Right. The tree feature was added in version 2.0.0 (again according > to the ChangeLog file). I guess you would prefer not making this a > requirement in tools/configure given the statement below. Right, I think we don't want to make a global requirement for yajl >= 2.0.0 (or 2.0.3) just yet so making xenstat fallback gracefully is probably the best option. > > Whereas a functional failure would perhaps be tolerable. However, given > > the existing HAVE_YAJL_YAJL_VERSION_H define I think the code could > > easily check if the YAJL library is good enough at compile time and stub > > itself out -- i.e. not report qdisk stats if the yajl doesn't do the > > job. > > Ok, I'll do it this way. Thanks. FWIW if HAVE_YAJL_YAJL_VERSION_H is not set then you can assume v1 (as libxl_json.h does). > > My second concern here is with the use of /var/run/xen/qmp-libxl-%i from > > outside of libxl. I can't remember if qemu is safe against multiple > > users of the socket. ISTR asking Anthony this before, but I don't recall > > the answer, sorry :-( > > > > Even if it is strictly speaking ok it seems a bit warty to do it, but > > perhaps for an in-tree user like libxenstat it is tolerable. > > Alternatively we could (relatively) easily arrange for their to be a > > second qemp-libxenstat-%i socket, assuming the qemu overhead of a second > > one is sane. > > As a test I modified libxl to create a qmp-libxenstat-%i socket and updated > libxenstat to use it instead of qmp-libxl-%i. It works fine although I don't > know if there is any performance penalty for having a second socket. I am > ok with going with this solution if this is preferred. I'm fine with this unless someone gives a good reason not to do it. > > Oh, I see my comments above were actually on the old code you were > > moving. > > I'll look at fixing this up based on your realloc comments above. Thanks! Ian.