Re: [Xen-devel] [PATCH,RFC]: Introduce libxl_domain_create()

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2010-12-14 at 19:00 +0000, Gianni Tedesco wrote:
> This patch is my initial attempt to re-factor the libxl domain creation
> paths to make them more suitable for external users and in particular
> the python bindings.
> 
> The patch is very rough and dirty at the moment, but that said it works
> for creating and restoring hvm domains at least. Migrate is not tested
> and I haven't made sure all the error handling and such like is correct.
> 
> Any thoughts or suggestions?
> 
> Gianni
> 
> # HG changeset patch
> # Parent 190c37e0eb307858c6cdf1bf5a0a5548babd2b34
> 
> diff -r 190c37e0eb30 tools/libxl/Makefile
> --- a/tools/libxl/Makefile      Tue Dec 14 18:00:35 2010 +0000
> +++ b/tools/libxl/Makefile      Tue Dec 14 18:54:20 2010 +0000
> @@ -20,7 +20,7 @@ ifeq ($(CONFIG_Linux),y)
>  LIBS += -luuid
>  endif
> 
> -LIBXL_OBJS-y = osdeps.o libxl_paths.o libxl_bootloader.o
> +LIBXL_OBJS-y = osdeps.o libxl_paths.o libxl_bootloader.o flexarray.o
>  ifeq ($(LIBXL_BLKTAP),y)
>  LIBXL_OBJS-y += libxl_blktap2.o
>  else
> @@ -29,7 +29,7 @@ endif
>  LIBXL_OBJS-$(CONFIG_X86) += libxl_cpuid.o
>  LIBXL_OBJS-$(CONFIG_IA64) += libxl_nocpuid.o
> 
> -LIBXL_OBJS = flexarray.o libxl.o libxl_pci.o libxl_dom.o libxl_exec.o 
> libxl_xshelp.o libxl_device.o libxl_internal.o libxl_utils.o $(LIBXL_OBJS-y)
> +LIBXL_OBJS = libxl.o libxl_create.o libxl_pci.o libxl_dom.o libxl_exec.o 
> libxl_xshelp.o libxl_device.o libxl_internal.o libxl_utils.o $(LIBXL_OBJS-y)
>  LIBXL_OBJS += _libxl_types.o

Not your fault but the distinction between LIBXL_OBJS-y and LIBXL_OBJS
is very non-obvious for things which are always included anyway, I'm not
sure it serves any purpose as it is.

>  AUTOINCS= libxlu_cfg_y.h libxlu_cfg_l.h
> diff -r 190c37e0eb30 tools/libxl/libxl.h
> --- a/tools/libxl/libxl.h       Tue Dec 14 18:00:35 2010 +0000
> +++ b/tools/libxl/libxl.h       Tue Dec 14 18:54:20 2010 +0000
> @@ -246,6 +246,37 @@ enum {
> 
>  #define LIBXL_VERSION 0
> 
> +enum action_on_shutdown {
> +    ACTION_DESTROY,
> +
> +    ACTION_RESTART,
> +    ACTION_RESTART_RENAME,
> +
> +    ACTION_PRESERVE,
> +
> +    ACTION_COREDUMP_DESTROY,
> +    ACTION_COREDUMP_RESTART,
> +};

Should be in the libxl namespace?

We probably need IDL support for enumerations and other constants.

> +typedef struct {
> +    libxl_domain_create_info c_info;
> +    libxl_domain_build_info b_info;
> +
> +    int num_disks, num_vifs, num_vif2s, num_pcidevs, num_vfbs,
> num_vkbs;
> +
> +    libxl_device_disk *disks;
> +    libxl_device_nic *vifs;
> +    libxl_device_net2 *vif2s;
> +    libxl_device_pci *pcidevs;
> +    libxl_device_vfb *vfbs;
> +    libxl_device_vkb *vkbs;
> +
> +    enum action_on_shutdown on_poweroff;
> +    enum action_on_shutdown on_reboot;
> +    enum action_on_shutdown on_watchdog;
> +    enum action_on_shutdown on_crash;
> +} libxl_domain_config;

Should be in IDL so it gets a destructor? Could require adding an Array
construct to handle the foo + num_foo style stuff.

> +static pid_t autoconnect_console(libxl_ctx *ctx, int domid)
> +{
[...]
> +}

I think console connect should be under toolstack control (i.e. stay in
xl). exec'ing the xenconsole client is only one way of connecting the
console, e.g. xapi might want to launch vncterm instead.

I think libxl_domain_create should take a callback function pointer to
connect the console. It's possible that libxl might also provide a
convenience function which launches xenconsole which is suitable for use
as this callback but ultimately it should be the toolstack's decision
what to do here.

> +
> +int libxl_domain_create(libxl_ctx *ctx, libxl_domain_config *d_config, 
> libxl_device_model_info *dm_info,

I think dm_info should be part of d_config.

> +                        unsigned int flags, int restore_fd, uint32_t 
> *domid_out)

For the external API I think I'd make the restore functionality a
separate function, even if it happens to call into the same internal
function.

libxl_domain_restore already exists but I guess the current
implementation, as well as libxl_domain_{make,build} and friends
could/should be make internal as part of this change.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel