Re: [libvirt] [PATCH v2 23/33] qemu: Use enum for CPU model expansion type

[Jiri Denemark]

On Tue, Feb 21, 2017 at 22:49:37 -0500, John Ferlan wrote:
...
> > @@ -5043,8 +5044,14 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr 
> > mon,
> >  if (virJSONValueObjectAppendString(model, "name", model_name) < 0)
> >  goto cleanup;
> >  
> > +switch (type) {
> > +case QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC:
> > +typeStr = "static";
> > +break;
> > +}
> > +
> 
> This works, but why not use VIR_ENUM_IMPL type logic?
> 
> Weak ACK, I think it'd be better to use VIR_ENUM_IMPL.

I explicitly wanted this to be visible in the context, mainly because
two different values are mapped to the same string.

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 23/33] qemu: Use enum for CPU model expansion type

[John Ferlan]

On 02/15/2017 11:44 AM, Jiri Denemark wrote:
> Signed-off-by: Jiri Denemark 
> ---
> 
> Notes:
> Version 2:
> - no change
> 
>  src/qemu/qemu_capabilities.c |  4 +++-
>  src/qemu/qemu_monitor.c  |  4 ++--
>  src/qemu/qemu_monitor.h  |  6 +-
>  src/qemu/qemu_monitor_json.c | 11 +--
>  src/qemu/qemu_monitor_json.h |  4 ++--
>  5 files changed, 21 insertions(+), 8 deletions(-)
> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index bcfb6b694..20aaaf8f0 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -2856,7 +2856,9 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCapsPtr qemuCaps,
>  model = "host";
>  }
>  
> -return qemuMonitorGetCPUModelExpansion(mon, "static", model, modelInfo);
> +return qemuMonitorGetCPUModelExpansion(mon,
> +   
> QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC,
> +   model, modelInfo);
>  }
>  
>  struct tpmTypeToCaps {
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index a434b234b..a987e107e 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -3641,11 +3641,11 @@ qemuMonitorCPUDefInfoFree(qemuMonitorCPUDefInfoPtr 
> cpu)
>  
>  int
>  qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon,
> -const char *type,
> +qemuMonitorCPUModelExpansionType type,
>  const char *model_name,
>  qemuMonitorCPUModelInfoPtr *model_info)
>  {
> -VIR_DEBUG("type=%s model_name=%s", type, model_name);
> +VIR_DEBUG("type=%d model_name=%s", type, model_name);
>  
>  QEMU_CHECK_MONITOR_JSON(mon);
>  
> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> index 112f041f1..b61f1cf54 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -948,8 +948,12 @@ struct _qemuMonitorCPUModelInfo {
>  qemuMonitorCPUPropertyPtr props;
>  };
>  
> +typedef enum {
> +QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC,
> +} qemuMonitorCPUModelExpansionType;
> +

If you went with VIR_ENUM_IMPL type logic for the strings, then you
could have a _UNKNOWN and _LAST too...

>  int qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon,
> -const char *type,
> +qemuMonitorCPUModelExpansionType type,
>  const char *model_name,
>  qemuMonitorCPUModelInfoPtr *model_info);
>  
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index d89dfb88d..dd7907482 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -5021,7 +5021,7 @@ qemuMonitorJSONParseCPUModelProperty(const char *key,
>  
>  int
>  qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
> -const char *type,
> +qemuMonitorCPUModelExpansionType type,
>  const char *model_name,
>  qemuMonitorCPUModelInfoPtr *model_info)
>  {
> @@ -5034,6 +5034,7 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
>  virJSONValuePtr cpu_props;
>  qemuMonitorCPUModelInfoPtr machine_model = NULL;
>  char const *cpu_name;
> +const char *typeStr = "";
>  
>  *model_info = NULL;
>  
> @@ -5043,8 +5044,14 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
>  if (virJSONValueObjectAppendString(model, "name", model_name) < 0)
>  goto cleanup;
>  
> +switch (type) {
> +case QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC:
> +typeStr = "static";
> +break;
> +}
> +

This works, but why not use VIR_ENUM_IMPL type logic?

Weak ACK, I think it'd be better to use VIR_ENUM_IMPL.

John
>  if (!(cmd = qemuMonitorJSONMakeCommand("query-cpu-model-expansion",
> -   "s:type", type,
> +   "s:type", typeStr,
> "a:model", model,
> NULL)))
>  goto cleanup;
> diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
> index 79688c82f..59d9f098c 100644
> --- a/src/qemu/qemu_monitor_json.h
> +++ b/src/qemu/qemu_monitor_json.h
> @@ -354,10 +354,10 @@ int qemuMonitorJSONGetCPUDefinitions(qemuMonitorPtr mon,
>  ATTRIBUTE_NONNULL(2);
>  
>  int qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
> -const char *type,
> +qemuMonitorCPUModelExpansionType 
> type,
>  const char *model_name,
>  qemuMonitorCPUModelInfoPtr 
> *model_info)
> -ATTRIBUTE_NONNULL(2) 

[libvirt] [PATCH v2 23/33] qemu: Use enum for CPU model expansion type

[Jiri Denemark]

Signed-off-by: Jiri Denemark 
---

Notes:
Version 2:
- no change

 src/qemu/qemu_capabilities.c |  4 +++-
 src/qemu/qemu_monitor.c  |  4 ++--
 src/qemu/qemu_monitor.h  |  6 +-
 src/qemu/qemu_monitor_json.c | 11 +--
 src/qemu/qemu_monitor_json.h |  4 ++--
 5 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index bcfb6b694..20aaaf8f0 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2856,7 +2856,9 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCapsPtr qemuCaps,
 model = "host";
 }
 
-return qemuMonitorGetCPUModelExpansion(mon, "static", model, modelInfo);
+return qemuMonitorGetCPUModelExpansion(mon,
+   
QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC,
+   model, modelInfo);
 }
 
 struct tpmTypeToCaps {
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index a434b234b..a987e107e 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3641,11 +3641,11 @@ qemuMonitorCPUDefInfoFree(qemuMonitorCPUDefInfoPtr cpu)
 
 int
 qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon,
-const char *type,
+qemuMonitorCPUModelExpansionType type,
 const char *model_name,
 qemuMonitorCPUModelInfoPtr *model_info)
 {
-VIR_DEBUG("type=%s model_name=%s", type, model_name);
+VIR_DEBUG("type=%d model_name=%s", type, model_name);
 
 QEMU_CHECK_MONITOR_JSON(mon);
 
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 112f041f1..b61f1cf54 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -948,8 +948,12 @@ struct _qemuMonitorCPUModelInfo {
 qemuMonitorCPUPropertyPtr props;
 };
 
+typedef enum {
+QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC,
+} qemuMonitorCPUModelExpansionType;
+
 int qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon,
-const char *type,
+qemuMonitorCPUModelExpansionType type,
 const char *model_name,
 qemuMonitorCPUModelInfoPtr *model_info);
 
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index d89dfb88d..dd7907482 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -5021,7 +5021,7 @@ qemuMonitorJSONParseCPUModelProperty(const char *key,
 
 int
 qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
-const char *type,
+qemuMonitorCPUModelExpansionType type,
 const char *model_name,
 qemuMonitorCPUModelInfoPtr *model_info)
 {
@@ -5034,6 +5034,7 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
 virJSONValuePtr cpu_props;
 qemuMonitorCPUModelInfoPtr machine_model = NULL;
 char const *cpu_name;
+const char *typeStr = "";
 
 *model_info = NULL;
 
@@ -5043,8 +5044,14 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
 if (virJSONValueObjectAppendString(model, "name", model_name) < 0)
 goto cleanup;
 
+switch (type) {
+case QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC:
+typeStr = "static";
+break;
+}
+
 if (!(cmd = qemuMonitorJSONMakeCommand("query-cpu-model-expansion",
-   "s:type", type,
+   "s:type", typeStr,
"a:model", model,
NULL)))
 goto cleanup;
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 79688c82f..59d9f098c 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -354,10 +354,10 @@ int qemuMonitorJSONGetCPUDefinitions(qemuMonitorPtr mon,
 ATTRIBUTE_NONNULL(2);
 
 int qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
-const char *type,
+qemuMonitorCPUModelExpansionType type,
 const char *model_name,
 qemuMonitorCPUModelInfoPtr *model_info)
-ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
+ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
 
 int qemuMonitorJSONGetCommands(qemuMonitorPtr mon,
char ***commands)
-- 
2.11.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list