Re: [PATCH] libxl: write IO ABI for disk frontends

["Pasi Kärkkäinen" <pasik@iki.fi>]

Hi,

adding Valtteri to CC because he originally reported the problem..

-- Pasi

On Tue, Apr 23, 2013 at 09:25:54PM +0100, Wei Liu wrote:
> This is a patch to forward-port a Xend behaviour. Xend writes IO ABI used for
> all frontends. Linux kernel before 2.6.26 relies on this behaviour otherwise
> it cannot boot. Blkfront after 2.6.26 writes that node itself, in which case
> it's just an overwrite of the existing node which should be OK.
> 
> In fact Xend writes the ABI for all frontends including console and vif. But
> nowadays it seems only old disk frontends rely on that ABI node so that we
> only write the ABI for disk frontends in libxl, minimizing the impact. Also
> ARM guests should not have this problem because they have new disk frontend so
> the snippets of this workaround are compiled for X86 target only.
> 
> Also cleaned up trailing whitespaces while I was there.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
>  tools/libxc/xc_domain.c     |   20 ++++++++++++++++++
>  tools/libxc/xenctrl.h       |   14 +++++++++++++
>  tools/libxl/libxl.c         |   12 +++++++++++
>  tools/libxl/libxl_create.c  |   49 +++++++++++++++++++++++++++++++++++++++++++
>  tools/libxl/libxl_types.idl |    7 ++++---
>  5 files changed, 99 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
> index 480ce91..c3d8e28 100644
> --- a/tools/libxc/xc_domain.c
> +++ b/tools/libxc/xc_domain.c
> @@ -379,6 +379,26 @@ int xc_domain_hvm_setcontext(xc_interface *xch,
>      return ret;
>  }
>  
> +#if defined (__i386__) || (__x86_64__)
> +/* get guest word width */
> +int xc_domain_getwidth(xc_interface *xch,
> +                       uint32_t domid,
> +                       uint32_t *guest_width)
> +{
> +    int ret;
> +    DECLARE_DOMCTL;
> +
> +    domctl.domain = domid;
> +    domctl.cmd = XEN_DOMCTL_get_address_size;
> +
> +    ret = do_domctl(xch, &domctl);
> +
> +    *guest_width = domctl.u.address_size.size;
> +
> +    return ret;
> +}
> +#endif
> +
>  int xc_vcpu_getcontext(xc_interface *xch,
>                         uint32_t domid,
>                         uint32_t vcpu,
> diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h
> index 50853af..c02dc1f 100644
> --- a/tools/libxc/xenctrl.h
> +++ b/tools/libxc/xenctrl.h
> @@ -636,6 +636,20 @@ int xc_domain_hvm_setcontext(xc_interface *xch,
>                               uint8_t *hvm_ctxt,
>                               uint32_t size);
>  
> +#if defined (__i386__) || (__x86_64__)
> +/**
> + * This function will return the guest word width
> + *
> + * @parm xch a handle to an open hypervisor interface
> + * @parm domid the domain to get address width for
> + * @parm guest_width word width for guest
> + * @return 0 on success, -ev on failure
> + */
> +int xc_domain_getwidth(xc_interface *xch,
> +                       uint32_t domid,
> +                       uint32_t *guest_width);
> +#endif
> +
>  /**
>   * This function returns information about the execution context of a
>   * particular vcpu of a domain.
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 572c2c6..5b8c9dc 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -2156,6 +2156,18 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
>          flexarray_append(front, "device-type");
>          flexarray_append(front, disk->is_cdrom ? "cdrom" : "disk");
>  
> +        /*
> +         * Old PV kernels before 2.6.26 rely on tool stack to write disk native
> +         * protocol to frontend node.
> +         *
> +         * New kernels write this node themselves. In that case it just
> +         * overwrites an existing node which is OK.
> +         */
> +        if (disk->native_protocol) {
> +            flexarray_append(front, "protocol");
> +            flexarray_append(front, disk->native_protocol);
> +        }
> +
>          libxl__device_generic_add(gc, t, device,
>                              libxl__xs_kvs_of_flexarray(gc, back, back->count),
>                              libxl__xs_kvs_of_flexarray(gc, front, front->count));
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 30a4507..7abcaf9 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -23,6 +23,7 @@
>  #include <xc_dom.h>
>  #include <xenguest.h>
>  #include <xen/hvm/hvm_info_table.h>
> +#include <xen/io/protocols.h>
>  
>  int libxl__domain_create_info_setdefault(libxl__gc *gc,
>                                           libxl_domain_create_info *c_info)
> @@ -910,6 +911,54 @@ static void domcreate_rebuild_done(libxl__egc *egc,
>          goto error_out;
>      }
>  
> +#if defined (__i386__) || (__x86_64__)
> +    /*
> +     * Pass along the disk navtive protocol to disks so that the protocol can
> +     * be written to frontend Xenstore node. This is a workaround for old PV
> +     * kernels before 2.6.26.
> +     *
> +     * Newer blkfront will write that node itself. In that case frontend just
> +     * rewrites the node as it sees fit.
> +     *
> +     * Note that Xend actually propagate this vaule to all frontends including
> +     * console and vifs. As now this is only needed for disk frontend so here
> +     * we minimize the impact.
> +     *
> +     * Presumably ARM guests don't have this problem, so this snippet is only
> +     * compiled for X86 target.
> +     */
> +
> +    if (d_config->c_info.type == LIBXL_DOMAIN_TYPE_PV) {
> +        int i;
> +        uint32_t guest_width;
> +        const char *protocol = NULL;
> +
> +        ret = xc_domain_getwidth(CTX->xch, domid, &guest_width);
> +        if (ret) {
> +            ret = ERROR_FAIL;
> +            goto error_out;
> +        }
> +
> +        switch (guest_width) {
> +        case 32: /* 32 bit guest */
> +            protocol = XEN_IO_PROTO_ABI_X86_32;
> +            break;
> +        case 64: /* 64 bit guest */
> +            protocol = XEN_IO_PROTO_ABI_X86_64;
> +            break;
> +        default:
> +            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> +                       "invalid address size for domain: %u",
> +                       guest_width);
> +            ret = ERROR_FAIL;
> +            goto error_out;
> +        }
> +
> +        for (i = 0; i < d_config->num_disks; i++)
> +            d_config->disks[i].native_protocol = strdup(protocol);
> +    }
> +#endif
> +
>      store_libxl_entry(gc, domid, &d_config->b_info);
>  
>      libxl__multidev_begin(ao, &dcs->multidev);
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index 6cb6de6..22622a9 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -274,7 +274,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
>      ("disable_migrate", libxl_defbool),
>      ("cpuid",           libxl_cpuid_policy_list),
>      ("blkdev_start",    string),
> -    
> +
>      ("device_model_version", libxl_device_model_version),
>      ("device_model_stubdomain", libxl_defbool),
>      # if you set device_model you must set device_model_version too
> @@ -318,9 +318,9 @@ libxl_domain_build_info = Struct("domain_build_info",[
>                                         ("keymap",           string),
>                                         ("sdl",              libxl_sdl_info),
>                                         ("spice",            libxl_spice_info),
> -                                       
> +
>                                         ("gfx_passthru",     libxl_defbool),
> -                                       
> +
>                                         ("serial",           string),
>                                         ("boot",             string),
>                                         ("usb",              libxl_defbool),
> @@ -371,6 +371,7 @@ libxl_device_disk = Struct("device_disk", [
>      ("removable", integer),
>      ("readwrite", integer),
>      ("is_cdrom", integer),
> +    ("native_protocol", string),
>      ])
>  
>  libxl_device_nic = Struct("device_nic", [
> -- 
> 1.7.10.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel