> On 18 Dec 2020, at 01:42, Elliott Mitchell <ehem+...@m5p.com> wrote:
>
> printf_info()/list_domains_details() had been serving fairly similar
> purposes. Increase their consistency (add file-handle and output_format
> arguments to list_domains_details(), reorder arguments) and then rename
> to better reflect their functionality.
>
> Both were simply outputting full domain information. As this is more of
> a dump operation, "dump" is a better name.
>
Looks ok to me
Reviewed-by: Luca Fancellu <luca.fance...@arm.com>
> Signed-off-by: Elliott Mitchell <ehem+...@m5p.com>
> ---
> tools/xl/xl.h | 8 ++++++++
> tools/xl/xl_info.c | 30 ++++++++++++++++--------------
> tools/xl/xl_misc.c | 5 +----
> 3 files changed, 25 insertions(+), 18 deletions(-)
>
> diff --git a/tools/xl/xl.h b/tools/xl/xl.h
> index 720adb0048..be5f4e11fe 100644
> --- a/tools/xl/xl.h
> +++ b/tools/xl/xl.h
> @@ -300,6 +300,14 @@ typedef enum {
> DOMAIN_RESTART_SOFT_RESET, /* Soft reset should be performed */
> } domain_restart_type;
>
> +extern void dump_by_config(enum output_format output_format,
> + FILE *fh,
> + const libxl_domain_config *d_config,
> + int domid);
> +extern void dump_by_dominfo_list(enum output_format output_format,
> + FILE *fh,
> + const libxl_dominfo info[],
> + int nb_domain);
> extern void printf_info_sexp(int domid, const libxl_domain_config *d_config,
> FILE *fh);
> extern void apply_global_affinity_masks(libxl_domain_type type,
> libxl_bitmap *vcpu_affinity_array,
> diff --git a/tools/xl/xl_info.c b/tools/xl/xl_info.c
> index 23d82ce2a2..3647468420 100644
> --- a/tools/xl/xl_info.c
> +++ b/tools/xl/xl_info.c
> @@ -94,12 +94,10 @@ out:
> return s;
> }
>
> -void printf_info(enum output_format output_format,
> - int domid,
> - libxl_domain_config *d_config, FILE *fh);
> -void printf_info(enum output_format output_format,
> - int domid,
> - libxl_domain_config *d_config, FILE *fh)
> +void dump_by_config(enum output_format output_format,
> + FILE *fh,
> + const libxl_domain_config *const d_config,
> + int domid)
> {
> if (output_format == OUTPUT_FORMAT_SXP)
> return printf_info_sexp(domid, d_config, fh);
> @@ -442,7 +440,10 @@ static void list_domains(bool verbose, bool context,
> bool claim, bool numa,
> libxl_physinfo_dispose(&physinfo);
> }
>
> -static void list_domains_details(const libxl_dominfo *info, int nb_domain)
> +void dump_by_dominfo_list(enum output_format output_format,
> + FILE *fh,
> + const libxl_dominfo info[],
> + int nb_domain)
> {
> libxl_domain_config d_config;
>
> @@ -453,7 +454,7 @@ static void list_domains_details(const libxl_dominfo
> *info, int nb_domain)
> const char *buf;
> libxl_yajl_length yajl_len = 0;
>
> - if (default_output_format == OUTPUT_FORMAT_JSON) {
> + if (output_format == OUTPUT_FORMAT_JSON) {
> hand = libxl_yajl_gen_alloc(NULL);
> if (!hand) {
> fprintf(stderr, "unable to allocate JSON generator\n");
> @@ -472,16 +473,16 @@ static void list_domains_details(const libxl_dominfo
> *info, int nb_domain)
> &d_config, NULL);
> if (rc)
> continue;
> - if (default_output_format == OUTPUT_FORMAT_JSON)
> + if (output_format == OUTPUT_FORMAT_JSON)
> s = printf_info_one_json(hand, info[i].domid, &d_config);
> else
> - printf_info_sexp(info[i].domid, &d_config, stdout);
> + printf_info_sexp(info[i].domid, &d_config, fh);
> libxl_domain_config_dispose(&d_config);
> if (s != yajl_gen_status_ok)
> goto out;
> }
>
> - if (default_output_format == OUTPUT_FORMAT_JSON) {
> + if (output_format == OUTPUT_FORMAT_JSON) {
> s = yajl_gen_array_close(hand);
> if (s != yajl_gen_status_ok)
> goto out;
> @@ -490,11 +491,12 @@ static void list_domains_details(const libxl_dominfo
> *info, int nb_domain)
> if (s != yajl_gen_status_ok)
> goto out;
>
> - puts(buf);
> + fputs(buf, fh);
> + fputc('\n', fh);
> }
>
> out:
> - if (default_output_format == OUTPUT_FORMAT_JSON) {
> + if (output_format == OUTPUT_FORMAT_JSON) {
> yajl_gen_free(hand);
> if (s != yajl_gen_status_ok)
> fprintf(stderr,
> @@ -571,7 +573,7 @@ int main_list(int argc, char **argv)
> }
>
> if (details)
> - list_domains_details(info, nb_domain);
> + dump_by_dominfo_list(default_output_format, stdout, info, nb_domain);
> else
> list_domains(verbose, context, false /* claim */, numa, cpupool,
> info, nb_domain);
> diff --git a/tools/xl/xl_misc.c b/tools/xl/xl_misc.c
> index 08f0fb6dc9..bcf178762b 100644
> --- a/tools/xl/xl_misc.c
> +++ b/tools/xl/xl_misc.c
> @@ -256,9 +256,6 @@ int main_dump_core(int argc, char **argv)
> return EXIT_SUCCESS;
> }
>
> -extern void printf_info(enum output_format output_format,
> - int domid,
> - libxl_domain_config *d_config, FILE *fh);
> int main_config_update(int argc, char **argv)
> {
> uint32_t domid;
> @@ -344,7 +341,7 @@ int main_config_update(int argc, char **argv)
> parse_config_data(filename, config_data, config_len, &d_config);
>
> if (debug || dryrun_only)
> - printf_info(default_output_format, -1, &d_config, stdout);
> + dump_by_config(default_output_format, stdout, &d_config, -1);
>
> if (!dryrun_only) {
> fprintf(stderr, "setting dom%u configuration\n", domid);
> --
> 2.30.2
>
>
