Re: [PATCH]: xl: move domain struct init functions to libxl

[Gianni Tedesco <gianni.tedesco@citrix.com>]

On Tue, 2011-01-11 at 12:53 +0000, Stefano Stabellini wrote:
> On Tue, 11 Jan 2011, Gianni Tedesco wrote:
> > This allows libxl users to get some sane default values for this complex
> > set of structures. This is purely code movement and there are no
> > functional changes.
> > 
> > Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>
> > 
> > 
> > diff -r feb198f3c97f tools/libxl/libxl.h
> > --- a/tools/libxl/libxl.h	Mon Jan 10 16:03:39 2011 +0000
> > +++ b/tools/libxl/libxl.h	Tue Jan 11 10:53:36 2011 +0000
> > @@ -280,6 +280,9 @@ int libxl_ctx_set_log(libxl_ctx *ctx, xe
> >  int libxl_ctx_postfork(libxl_ctx *ctx);
> >  
> >  /* domain related functions */
> > +void libxl_init_create_info(libxl_domain_create_info *c_info);
> > +void libxl_init_build_info(libxl_domain_build_info *b_info, libxl_domain_create_info *c_info);
> > +void libxl_init_dm_info(libxl_device_model_info *dm_info, libxl_domain_create_info *c_info, libxl_domain_build_info *b_info);
> >  typedef int (*libxl_console_ready)(libxl_ctx *ctx, uint32_t domid, void *priv);
> >  int libxl_domain_create_new(libxl_ctx *ctx, libxl_domain_config *d_config, libxl_console_ready cb, void *priv, uint32_t *domid);
> >  int libxl_domain_create_restore(libxl_ctx *ctx, libxl_domain_config *d_config, libxl_console_ready cb, void *priv, uint32_t *domid, int restore_fd);
> > diff -r feb198f3c97f tools/libxl/libxl_create.c
> 
> 
> What about init_nic_info, init_net2_info, init_vfb_info, init_vkb_info
> and init_console_info?
> Wouldn't make sense to have a libxl_init_domain_config

Yeah good point, I can re-spin to include the various device info's.

Not sure about an init_domain_config, I don't see how it would work,
currently we have:

libxl_domain_config x;
init_create_info(&x.c_info);
// do stuff to c_info
init_build_info(&x.b_info, &x.c_info);
// do stuff to b_info
init_dm_info(&x.dm_info, &x.c_info, &x.b_info);
// do stuff to dm_info
for each device { init device; do stuff to device; }

You could init them all in one functions but it breaks IMO the main
purpose of these functions which is that when you 'do stuff' to b_info
then init_dm_info() depends on that to set the sane defaults (eg. for
vidmem).

So either we would have every field as a parameter to this function or
add callbacks for the 'do stuff' bits which seems a very rigid
interface. I think we should wait for 4.2 before we try to make any
more-sweeping changes in this area.

Gianni