Re: [Qemu-devel] [Qemu-ppc] [PATCH v5 1/2] spapr: helper functions to get valid host fields

[Greg Kurz]

On Mon, 11 Mar 2019 19:57:08 -0300
"Maxiwell S. Garcia" <address@hidden> wrote:

> The pseries options 'host-serial' and 'host-model' accepts
> 'none', 'passthrough', or <string> content. The helper
> functions in this commit return a valid host field based on
> user options.
> 
> Signed-off-by: Maxiwell S. Garcia <address@hidden>
> ---
>  hw/ppc/spapr.c         | 58 ++++++++++++++++++++++++++----------------
>  include/hw/ppc/spapr.h |  3 +++
>  2 files changed, 39 insertions(+), 22 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 9e01226e18..a3078f0261 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1202,6 +1202,34 @@ static void spapr_dt_chosen(sPAPRMachineState *spapr, 
> void *fdt)
>      g_free(bootlist);
>  }
>  
> +char *spapr_get_valid_host_serial(sPAPRMachineState *spapr)
> +{
> +    char *host_serial = NULL;
> +    if (spapr->host_serial && !g_str_equal(spapr->host_serial, "none")) {
> +        if (g_str_equal(spapr->host_serial, "passthrough")) {
> +            /* -M host-serial=passthrough */
> +            kvmppc_get_host_serial(&host_serial);
> +        } else {
> +            host_serial = g_strdup(spapr->host_serial);
> +        }
> +    }
> +    return host_serial;
> +}
> +
> +char *spapr_get_valid_host_model(sPAPRMachineState *spapr)
> +{
> +    char *host_model = NULL;
> +    if (spapr->host_model && !g_str_equal(spapr->host_model, "none")) {
> +        if (g_str_equal(spapr->host_model, "passthrough")) {
> +            /* -M host-model=passthrough */
> +            kvmppc_get_host_model(&host_model);
> +        } else {
> +            host_model = g_strdup(spapr->host_model);
> +        }
> +    }
> +    return host_model;
> +}
> +

These two functions only differ because of the host or serial wording.
Maybe consolidate the boiler plate to a macro ?

>  static void spapr_dt_hypervisor(sPAPRMachineState *spapr, void *fdt)
>  {
>      /* The /hypervisor node isn't in PAPR - this is a hack to allow PR
> @@ -1247,30 +1275,16 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr)
>       * Add info to guest to indentify which host is it being run on
>       * and what is the uuid of the guest
>       */
> -    if (spapr->host_model && !g_str_equal(spapr->host_model, "none")) {
> -        if (g_str_equal(spapr->host_model, "passthrough")) {
> -            /* -M host-model=passthrough */
> -            if (kvmppc_get_host_model(&buf)) {
> -                _FDT(fdt_setprop_string(fdt, 0, "host-model", buf));
> -                g_free(buf);
> -            }
> -        } else {
> -            /* -M host-model=<user-string> */
> -            _FDT(fdt_setprop_string(fdt, 0, "host-model", 
> spapr->host_model));
> -        }
> +    buf = spapr_get_valid_host_model(spapr);
> +    if (buf) {
> +        _FDT(fdt_setprop_string(fdt, 0, "host-model", buf));
> +        g_free(buf);
>      }
>  
> -    if (spapr->host_serial && !g_str_equal(spapr->host_serial, "none")) {
> -        if (g_str_equal(spapr->host_serial, "passthrough")) {
> -            /* -M host-serial=passthrough */
> -            if (kvmppc_get_host_serial(&buf)) {
> -                _FDT(fdt_setprop_string(fdt, 0, "host-serial", buf));
> -                g_free(buf);
> -            }
> -        } else {
> -            /* -M host-serial=<user-string> */
> -            _FDT(fdt_setprop_string(fdt, 0, "host-serial", 
> spapr->host_serial));
> -        }
> +    buf = spapr_get_valid_host_serial(spapr);
> +    if (buf) {
> +        _FDT(fdt_setprop_string(fdt, 0, "host-serial", buf));
> +        g_free(buf);
>      }
>  
>      buf = qemu_uuid_unparse_strdup(&qemu_uuid);
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 59073a7579..f7ea99dc69 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -842,6 +842,9 @@ int spapr_caps_post_migration(sPAPRMachineState *spapr);
>  
>  void spapr_check_pagesize(sPAPRMachineState *spapr, hwaddr pagesize,
>                            Error **errp);
> +
> +char *spapr_get_valid_host_serial(sPAPRMachineState *spapr);
> +char *spapr_get_valid_host_model(sPAPRMachineState *spapr);
>  /*
>   * XIVE definitions
>   */