Re: [Xen-devel] [PATCH 1/4] libxl: populate build_info vfb in separate function

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Fri, May 08, 2015 at 04:31:02PM -0600, Jim Fehlig wrote:
> For HVM domains, vfb info must be populated in the libxl_domain_build_info
> stuct.  Currently this is done in the libxlMakeVfbList function, but IMO

struct
> it would be cleaner to populate the build_info vfb in a separate
> libxlMakeBuildInfoVfb function.  libxlMakeVfbList would then handle only
> vfb devices, simiar to the other libxlMake<device>List functions.
> 
> A future patch will extend libxlMakeBuildInfoVfb to support SPICE.
> 
> Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx>
> ---
>  src/libxl/libxl_conf.c | 79 
> ++++++++++++++++++++++++++++++--------------------
>  1 file changed, 48 insertions(+), 31 deletions(-)
> 
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index fccada5..8552c77 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -1323,37 +1323,6 @@ libxlMakeVfbList(virPortAllocatorPtr graphicsports,
>      d_config->vkbs = x_vkbs;
>      d_config->num_vfbs = d_config->num_vkbs = nvfbs;
>  
> -    /*
> -     * VNC or SDL info must also be set in libxl_domain_build_info
> -     * for HVM domains.  Use the first vfb device.
> -     */
> -    if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) {
> -        libxl_domain_build_info *b_info = &d_config->b_info;
> -        libxl_device_vfb vfb = d_config->vfbs[0];
> -
> -        if (libxl_defbool_val(vfb.vnc.enable)) {
> -            libxl_defbool_set(&b_info->u.hvm.vnc.enable, true);
> -            if (VIR_STRDUP(b_info->u.hvm.vnc.listen, vfb.vnc.listen) < 0)
> -                goto error;
> -            if (VIR_STRDUP(b_info->u.hvm.vnc.passwd, vfb.vnc.passwd) < 0)
> -                goto error;
> -            b_info->u.hvm.vnc.display = vfb.vnc.display;
> -            libxl_defbool_set(&b_info->u.hvm.vnc.findunused,
> -                              libxl_defbool_val(vfb.vnc.findunused));
> -        } else if (libxl_defbool_val(vfb.sdl.enable)) {
> -            libxl_defbool_set(&b_info->u.hvm.sdl.enable, true);
> -            libxl_defbool_set(&b_info->u.hvm.sdl.opengl,
> -                              libxl_defbool_val(vfb.sdl.opengl));
> -            if (VIR_STRDUP(b_info->u.hvm.sdl.display, vfb.sdl.display) < 0)
> -                goto error;
> -            if (VIR_STRDUP(b_info->u.hvm.sdl.xauthority, vfb.sdl.xauthority) 
> < 0)
> -                goto error;
> -        }
> -
> -        if (VIR_STRDUP(b_info->u.hvm.keymap, vfb.keymap) < 0)
> -            goto error;
> -    }
> -
>      return 0;
>  
>   error:
> @@ -1367,6 +1336,51 @@ libxlMakeVfbList(virPortAllocatorPtr graphicsports,
>  }
>  
>  /*
> + * Populate vfb info in libxl_domain_build_info struct for HVM domains.
> + * Use first libxl_device_vfb device in libxl_domain_config->vfbs.
> + * Prior to calling this function, libxlMakeVfbList must be called to
> + * populate libxl_domain_config->vfbs.
> + */
> +static int
> +libxlMakeBuildInfoVfb(virDomainDefPtr def, libxl_domain_config *d_config)
> +{
> +    libxl_domain_build_info *b_info = &d_config->b_info;
> +    libxl_device_vfb x_vfb;
> +
> +    if (def->os.type != VIR_DOMAIN_OSTYPE_HVM)
> +        return 0;
> +
> +    if (d_config->num_vfbs == 0)
> +        return 0;
> +
> +    x_vfb = d_config->vfbs[0];
> +
> +    if (libxl_defbool_val(x_vfb.vnc.enable)) {
> +        libxl_defbool_set(&b_info->u.hvm.vnc.enable, true);
> +        if (VIR_STRDUP(b_info->u.hvm.vnc.listen, x_vfb.vnc.listen) < 0)
> +            return -1;
> +        if (VIR_STRDUP(b_info->u.hvm.vnc.passwd, x_vfb.vnc.passwd) < 0)
> +            return -1;
> +        b_info->u.hvm.vnc.display = x_vfb.vnc.display;
> +        libxl_defbool_set(&b_info->u.hvm.vnc.findunused,
> +                          libxl_defbool_val(x_vfb.vnc.findunused));
> +    } else if (libxl_defbool_val(x_vfb.sdl.enable)) {
> +        libxl_defbool_set(&b_info->u.hvm.sdl.enable, true);
> +        libxl_defbool_set(&b_info->u.hvm.sdl.opengl,
> +                          libxl_defbool_val(x_vfb.sdl.opengl));
> +        if (VIR_STRDUP(b_info->u.hvm.sdl.display, x_vfb.sdl.display) < 0)
> +            return -1;
> +        if (VIR_STRDUP(b_info->u.hvm.sdl.xauthority, x_vfb.sdl.xauthority) < 
> 0)
> +            return -1;
> +    }
> +
> +    if (VIR_STRDUP(b_info->u.hvm.keymap, x_vfb.keymap) < 0)
> +        return -1;
> +
> +    return 0;
> +}
> +
> +/*
>   * Get domain0 autoballoon configuration.  Honor user-specified
>   * setting in libxl.conf first.  If not specified, autoballooning
>   * is disabled when domain0's memory is set with 'dom0_mem'.
> @@ -1764,6 +1778,9 @@ libxlBuildDomainConfig(virPortAllocatorPtr 
> graphicsports,
>      if (libxlMakeVfbList(graphicsports, def, d_config) < 0)
>          return -1;
>  
> +    if (libxlMakeBuildInfoVfb(def, d_config) < 0)
> +        return -1;
> +
>      if (libxlMakePCIList(def, d_config) < 0)
>          return -1;
>  
> -- 
> 1.8.4.5
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

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