[Xen-devel] Re: [PATCH UPDATED] libxl: enabling upstream qemu as pure pv backend.

[Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>]

On Tue, 2011-07-19 at 06:53 +0100, Wei Liu wrote:
> diff -r 809882b6f1f8 -r fd24899c2558 tools/libxl/libxl_create.c
> --- a/tools/libxl/libxl_create.c        Mon Jul 18 14:52:30 2011 +0100
> +++ b/tools/libxl/libxl_create.c        Tue Jul 19 13:51:39 2011 +0800
> @@ -530,6 +530,7 @@
>      {
>          int need_qemu = 0;
>          libxl_device_console console;
> +        libxl_device_model_info xenpv_dm_info;
> 
>          for (i = 0; i < d_config->num_vfbs; i++) {
>              libxl_device_vfb_add(ctx, domid, &d_config->vfbs[i]);
> @@ -550,8 +551,19 @@

Please can you add to your ~/.hgrc:
        [diff]
        showfunc = True

I've updated http://wiki.xen.org/xenwiki/SubmittingXenPatches with it.

>          libxl__device_console_add(gc, domid, &console, &state);
>          libxl_device_console_destroy(&console);
> 
> +        /* only copy those useful configs */
> +        memset((void*)&xenpv_dm_info, 0x00, sizeof(libxl_device_model_info));
> +        xenpv_dm_info.device_model_version =
> +            d_config->dm_info.device_model_version;
> +        xenpv_dm_info.type = d_config->dm_info.type;
> +        xenpv_dm_info.device_model = d_config->dm_info.device_model;
> +        xenpv_dm_info.extra = d_config->dm_info.extra;
> +        xenpv_dm_info.extra_pv = d_config->dm_info.extra_pv;
> +        xenpv_dm_info.extra_hvm = d_config->dm_info.extra_hvm;

All this could be under the following if ?

>          if (need_qemu)
> -            libxl__create_xenpv_qemu(gc, domid, d_config->vfbs, 
> &dm_starting);
> +            libxl__create_xenpv_qemu(gc, domid, &xenpv_dm_info,
> +                                     d_config->vfbs, &dm_starting);
> 
> @@ -742,7 +751,14 @@
>          if (ret)
>              goto out_free;
>      }
> -    if (libxl__create_xenpv_qemu(gc, domid, vfb, &dm_starting) < 0) {
> +
> +    memset((void*)&xenpv_dm_info, 0x00, sizeof(libxl_device_model_info));
> +    xenpv_dm_info.device_model_version = info->device_model_version;
> +    xenpv_dm_info.type = LIBXL_DOMAIN_TYPE_PV;
> +    xenpv_dm_info.device_model = info->device_model;

Why don't extra_* get copied here?

Perhaps this and the other similar hunk above could be split into a
common libxl__init_xenpv_dm_info(&xenpv_dm_info, info)?

If there is a reason for the extra_* difference then add an "int
is_stubdom" to the init function and gate accordingly?

That said, I've no objection to this patch going in as is and addressing
comments later as necessary.

Ian.



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