Re: [Xen-devel] [PATCH-RFC] Change libxl to use Xen 4.2 interface

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2012-05-09 at 18:13 +0100, Daniel De Graaf wrote:
> This patch changes libxl to use the interface in Xen 4.2. It is provided
> as an example, not intended to go in to libvirt as-is since it removes
> all support for libxl from Xen 4.1. It also still has some cruft (extra
> void casts on parameters) and the device model info population is not
> written. It has been tested with simple domain create/destroy.
> ---
>  src/libxl/libxl_conf.c   |  134 +++++++++------
>  src/libxl/libxl_conf.h   |    8 +-
>  src/libxl/libxl_driver.c |  430 
> ++++++++++++++++++++++++++--------------------
>  3 files changed, 326 insertions(+), 246 deletions(-)

Thanks Daniel, interesting to see. It doesn't seem as invasive as I
expected given the number of changes to libxl's interfaces between 4.1
and 4.2. 

I suppose it is worth mentioning in this context that we are intending
to maintain the 4.2 libxl API in a stable manner going forward, as
described near the top of:
http://xenbits.xen.org/hg/staging/xen-unstable.hg/file/tip/tools/libxl/libxl.h

Since libvirt is one of the main reasons we are doing this it would be
useful to check that this actually meets the needs from that side.

That doesn't really help with support 4.1 and 4.2+. A large portion of
the below looks like it would be reasonably easy to abstract away with a
header full of compat defines for naming changes etc, other bits don't
look so simple to deal with.

> @@ -403,9 +414,9 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, 
> libxl_domain_config *d_config)
>          return -1;
>      }
> 
> -    libxl_init_build_info(b_info, &d_config->c_info);
> +    libxl_domain_build_info_init(b_info);
> 
> -    b_info->hvm = hvm;
> +    b_info->type = hvm ? LIBXL_DOMAIN_TYPE_HVM : LIBXL_DOMAIN_TYPE_PV;

I'm not sure what version of "4.2" you are currently using but this
should become libxl_domain_build_info_init_type(b_info, hvm ? LIBXL...)
at some point.

>      b_info->max_vcpus = def->maxvcpus;
>      if (def->vcpus == 32)
>          b_info->cur_vcpus = (uint32_t) -1;
[...]
> @@ -454,7 +466,14 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, 
> libxl_domain_config *d_config)
>              }
>          }
>          if (def->os.bootloaderArgs) {
> -            if ((b_info->u.pv.bootloader_args = 
> strdup(def->os.bootloaderArgs)) == NULL) {
> +            // XXX may need to split these arguments on a delimiter

I think you do. We could consider moving split_string_into_string_list
from xl into the xlu(tility) library if libvirt doesn't have an existing
helper for that, or you can just copy it (I'm the sole author of that
function, I'd be happy to ack any reasonable license for use in libvirt,
some variant of (L)GPL I presume?)

[...]
> +static const libxl_osevent_hooks event_callbacks = {
> +    .fd_register = evhook_fd_register,
> +    .fd_modify = evhook_fd_modify,
> +    .fd_deregister = evhook_fd_deregister,
> +    .timeout_register = evhook_timeout_register,
> +    .timeout_modify = evhook_timeout_modify,
> +    .timeout_deregister = evhook_timeout_deregister,
> +};

Glad to see that this interface seems to fit in well with libvirt's
infrastructure -- that was the point of it after all ;-)

> +static const struct libxl_event_hooks ev_hooks = {
> +    .event_occurs_mask = LIBXL_EVENTMASK_ALL,
> +    .event_occurs = libxlEventHandler,
> +    .disaster = NULL,
> +};

Same for this interface.

Ian.


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