On Tue, Mar 12, 2019 at 11:52:24AM +0100, Greg Kurz wrote: Hi Greg,
> On Mon, 11 Mar 2019 19:57:08 -0300 > "Maxiwell S. Garcia" <maxiw...@linux.ibm.com> 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 <maxiw...@linux.ibm.com> > > --- > > 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 ? > Do you suggest something like that? #define SPAPR_GET_VALID_HOST_(FIELD, buf) \ if (spapr->FIELD && !g_str_equal(spapr->FIELD, "none")) { \ if (g_str_equal(spapr->FIELD, "passthrough")) { \ kvmppc_get_##FIELD(&buf); \ } else { \ buf = g_strdup(spapr->FIELD); \ } \ } \ #define SPAPR_GET_VALID_HOST_SERIAL(buf) SPAPR_GET_VALID_HOST_(host_serial, buf) #define SPAPR_GET_VALID_HOST_MODEL(buf) SPAPR_GET_VALID_HOST_(host_model, buf) > > 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 > > */ >