Re: [PATCH v2] xentop: add support for qdisks

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

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.