Re: [Xen-devel] [PATCH] xl: Change output from xl -N create to be more useful

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Fri, Jun 26, 2015 at 03:29:15PM +0100, Ian Jackson wrote:
> Currently, xl -N create produces:
> 
>     {
>         "domid": null,
>         "config": {
>             "c_info": {
>                 "type": "pv",
>      [etc]
>     }
> 
> The domid is always NULL (as the domain has not been created at this
> stage).
> 
> This is annoying if you want to take this output and use it for some
> actually useful purpose like domain creation: either it needs to be
> massaged, or the the consuming tool needs to be taught to look inside
> the json object for the `config' element (which IMO makes no sense as
> an interface).
> 
> We would like to be able to pass libxl json configs around sensibly.
> In the future maybe xl will grow an option to create a domain from a
> json config, and this is currently something I want to be able to have
> a test tool do.
> 
> Note that this change is NOT BACKWARDS COMPATIBLE.  But it would only
> adversely affects anyone who uses `xl -N create' and then saves and
> processes the JSON.  (The output from xl list et al is not changed; it
> normally needs the domid.)  Such a user should probably have already
> have complained about the infelicitous output.  If they haven't it
> would be simple enough for them to bookend the output so as to provide
> compatible output.
> 
> If this backward compatibility problem is considered a blocker for
> this patch, then I will respin, with one of the following two
> workarounds:
>   - A new option to force sane output
>   - Generate output which contains the domain config twice,
>     once directly in the main struct, and a copy in "config"

I don't think keeping a broken interface for the sake of backward
compatibility is worth it.

> 
> Signed-off-by: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
> CC: Ian Campbell <ian.campbell@xxxxxxxxxx>
> CC: Wei Liu <wei.liu2@xxxxxxxxxx>
> CC: Euan Harris <euan.harris@xxxxxxxxxx>
> ---
>  tools/libxl/xl_cmdimpl.c |   17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index c858068..9e9ee5e 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -2687,9 +2687,20 @@ static uint32_t create_domain(struct domain_create 
> *dom_info)
>          }
>      }
>  
> -    if (debug || dom_info->dryrun)
> -        printf_info(default_output_format, -1, &d_config,
> -                    debug ? stderr : stdout);
> +    if (debug || dom_info->dryrun) {
> +        FILE *cfg_print_fh = debug ? stderr : stdout;
> +        if (default_output_format == OUTPUT_FORMAT_SXP) {
> +            printf_info_sexp(-1, &d_config, cfg_print_fh);
> +        } else {
> +            char *json = libxl_domain_config_to_json(ctx, &d_config);
> +            fputs(json, stdout);
> +            free(json);
> +            if (ferror(stdout) || fflush(stdout)) {
> +                perror("stdout"); exit(-1);
> +            }
> +        }
> +    }
> +
>  

Actually you may want to update main_config_update, which also prints
out domain configuration. Then remove the will-be-defunct
printf_info{,_one_json}.

Wei.

>      ret = 0;
>      if (dom_info->dryrun)
> -- 
> 1.7.10.4

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