Re: [Xen-devel] [PATCH v1 09/10] libxl: introduce libxl_retrieve_domain_configuration

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Thu, Jul 17, 2014 at 03:02:11PM +0100, Ian Campbell wrote:
> 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?

That will do for me. But as I'm the one implementing this I can always
understand the description however vague it is. :-)

> > 
> > > > 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
> 

So I was thinking about "slow" in general while you were talking about
"slow" in the context of libxl.

If we discuss this in the context of libxl, then I think this API is a
"fast" API that doesn't require to be made async.

> > > > +    /* 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. </pendant>
> > > 
> > 
> > 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?
> 

I'm fine with this.

Wei.

> Ian.
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel