Re: [PATCH 3/5] tools/xl: Rename printf_info()/list_domains_details() to dump_by_...()

[Luca Fancellu <luca.fancellu@xxxxxxx>]

> On 18 Dec 2020, at 01:42, Elliott Mitchell <ehem+xen@xxxxxxx> 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.fancellu@xxxxxxx>

> Signed-off-by: Elliott Mitchell <ehem+xen@xxxxxxx>
> ---
> 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
> 
>