From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH v1 09/10] libxl: introduce libxl_retrieve_domain_configuration Date: Thu, 17 Jul 2014 15:02:11 +0100 Message-ID: <1405605731.31127.25.camel@kazak.uk.xensource.com> References: <1405002745-5034-1-git-send-email-wei.liu2@citrix.com> <1405002745-5034-10-git-send-email-wei.liu2@citrix.com> <1405594741.12538.15.camel@kazak.uk.xensource.com> <20140717121143.GK4913@zion.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20140717121143.GK4913@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-07-17 at 13:11 +0100, Wei Liu wrote: > On Thu, Jul 17, 2014 at 11:59:01AM +0100, Ian Campbell wrote: > > On Thu, 2014-07-10 at 15:32 +0100, Wei Liu wrote: > > > Introduce a new public API to return domain configuration. This returned > > > configuration can be used to rebuild a domain. > > > > > > Note that this configuration doesn't equal to the current state of the > > > domain. What it does is to use JSON version configuration as template > > > and pull in relevant information from xenstore. > > > > I think this configuration does equal the current state, doesn't it? > > Isn't that the whole point? > > > > Hrm... by "current state" I mean the current running state. But to > rebuild a domain we might leave some configurations for the remote host > toolstack to decide. The configuration this API returns is (template > configuration + current state that we care about). Hrm, I can't think of a good name for this. Describing it as the state needed to reproduce the guest visible state might make sense? > > > > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c > > > index 49d7ef6..cd5914c 100644 > > > --- a/tools/libxl/libxl.c > > > +++ b/tools/libxl/libxl.c > > > @@ -6034,6 +6034,206 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, libxl_mac *src) > > > for (i = 0; i < 6; i++) > > > (*dst)[i] = (*src)[i]; > > > } > > > + > > > +int libxl_retrieve_domain_configuration(libxl_ctx *ctx, uint32_t domid, > > > + libxl_domain_config *d_config) > > > > It occurs to me to wonder if any function which takes a lock ought to be > > async capable? > > > > Good point. > > > Also is there is any possibility that any of the operations needed to > > gather the updated configuration might take a long time. > > > > I think reading from xenstore and filesystem can be slow, in the sense > that if there's much contention on these two resources it could take > seconds to finish operations. I'm pretty sure we don't consider those to be "slow". See in libxl_internal.h: * "Slow" functions includes any that might block on a guest or an * external script. More broadly, it includes any operations which * are sufficiently slow that an application might reasonably want to * initiate them, and then carry on doing something else, while the * operation completes. That is, a "fast" function must be fast * enough that we do not mind blocking all other management operations * on the same host while it completes. * * There are certain primitive functions which make a libxl operation * necessarily "slow" for API reasons. These are: * - awaiting xenstore watches (although read-modify-write xenstore * transactions are OK for fast functions) * - spawning subprocesses * - anything with a timeout > > > + /* Memory limits: > > > + * > > > + * Currently there're three memory limits: > > > + * 1. "target" in xenstore (originally memory= in config file) > > > + * 2. "static-max" in xenstore (originally maxmem= in config file) > > > > Nit: strictly speaking those "originally..." are xl specific and this is > > libxl. > > > > You mean I should write "originally memory= in xl config file"? That would satisfy my pedantry, yes ;-) But maybe you want to reference the libxl_domain_config fields instead? Ian.