1. Patchew
  2. QEMU
  3. [Qemu-devel] [PATCH v5 0/2] spapr-rtas: add ibm, get-vpd RTAS interface
  4. View patch

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

Maxiwell S. Garcia posted 2 patches 6 years, 11 months ago
Maintainers: David Gibson <david@gibson.dropbear.id.au>
There is a newer version of this series
[Qemu-devel] [PATCH v5 1/2] spapr: helper functions to get valid host fields
Posted by Maxiwell S. Garcia 6 years, 11 months ago 
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 <maxiwell@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;
+}
+
 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
  */
-- 
2.20.1
Re: [Qemu-devel] [Qemu-ppc] [PATCH v5 1/2] spapr: helper functions to get valid host fields
Posted by Greg Kurz 6 years, 11 months ago 
On Mon, 11 Mar 2019 19:57:08 -0300
"Maxiwell S. Garcia" <maxiwell@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 <maxiwell@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 ?

>  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
>   */
Re: [Qemu-devel] [Qemu-ppc] [PATCH v5 1/2] spapr: helper functions to get valid host fields
Posted by Maxiwell S. Garcia 6 years, 11 months ago 
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" <maxiwell@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 <maxiwell@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
> >   */
>
Re: [Qemu-devel] [Qemu-ppc] [PATCH v5 1/2] spapr: helper functions to get valid host fields
Posted by Greg Kurz 6 years, 11 months ago 
On Wed, 13 Mar 2019 18:16:18 -0300
"Maxiwell S. Garcia" <maxiwell@linux.ibm.com> wrote:

> On Tue, Mar 12, 2019 at 11:52:24AM +0100, Greg Kurz wrote:
> 
> Hi Greg,
> 

Hi Maxiwell,

> > On Mon, 11 Mar 2019 19:57:08 -0300
> > "Maxiwell S. Garcia" <maxiwell@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 <maxiwell@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)
> 

Yeah that's the idea. I was more specifically thinking to
something like:

#define SPAPR_GET_VALID_HOST(attr)                            \
char *spapr_get_valid_host_##attr(sPAPRMachineState *spapr)   \
{                                                             \
    char *host_##attr = NULL;                                 \
    if (spapr->host_##attr &&                                 \
        !g_str_equal(spapr->host_##attr, "none")) {           \
        if (g_str_equal(spapr->host_##attr, "passthrough")) { \
            kvmppc_get_host_##attr(&host_##attr);             \
        } else {                                              \
            host_##attr = g_strdup(spapr->host_##attr);       \
        }                                                     \
    }                                                         \
    return host_##attr;                                       \
}

SPAPR_GET_VALID_HOST(serial);
SPAPR_GET_VALID_HOST(model);

> > >  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
> > >   */  
> >   
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.