From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH] xl: Change output from xl -N create to be more useful Date: Fri, 3 Jul 2015 12:25:57 +0100 Message-ID: <1435922757.9447.92.camel@citrix.com> References: <1435328955-19744-1-git-send-email-ian.jackson@eu.citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1435328955-19744-1-git-send-email-ian.jackson@eu.citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Jackson Cc: Euan Harris , xen-devel@lists.xensource.com, Wei Liu List-Id: xen-devel@lists.xenproject.org On Fri, 2015-06-26 at 15:29 +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" > > Signed-off-by: Ian Jackson > CC: Ian Campbell > CC: Wei Liu > CC: Euan Harris > --- > 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; Did you mean to use this instead of the hardcoded stdout below? Otherwise the debug output's location differs depending on the format, which seems unexpected. > + 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); > + } > + } > + } > + > > ret = 0; > if (dom_info->dryrun)