9:55 p.m.
This is part of resolution of: https://bugzilla.redhat.com/show_bug.cgi?id=1511999
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
diff to v1:
- Replaced c++ style comments with c style
- qemuMonitorJSONGetCPUModelInfo{ToJSON,FromJSON} use To/From form
- qemuMonitorJSONGetCPUModelInfo{ToJSON,FromJSON} return pointers not integers
- VIR_STEAL_PTR form used
- switch statement uses virReportEnumRangeError
- qemuMonitorJSONHasError(reply, "GenericError") treated as no info, not fatal
error
- virJSONValueFree(cpu_props) during error case
diff to v2:
- qemuMonitorJSONGetCPUModelInfo{ToJSON,FromJSON}
works on {"name": "IvyBridge", "props": {}}
rather than {"model": {"name": "IvyBridge",
"props": {}}}
- additional cleanups and fixes
Chris Venteicher (3):
qemu_monitor_json: Populate CPUModelInfo struct from json
qemu_monitor_json: Build Json CPU Model Info
qemu_monitor: query-cpu-model-baseline QMP command
src/qemu/qemu_monitor.c | 13 ++++
src/qemu/qemu_monitor.h | 5 ++
src/qemu/qemu_monitor_json.c | 179 +++++++++++++++++++++++++++++++++++++------
src/qemu/qemu_monitor_json.h | 7 ++
4 files changed, 182 insertions(+), 22 deletions(-)
--
2.14.1
9:55 p.m.
New subject: [libvirt] [PATCHv3 1/3] qemu_monitor_json: Populate CPUModelInfo struct from json
Extract cpu_model parsing code from qemuMonitorJSONGetCPUModelExpansion
into a separate helper for future reuse.
---
src/qemu/qemu_monitor_json.c | 77 +++++++++++++++++++++++++++-----------------
1 file changed, 47 insertions(+), 30 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 57c2c4de0..83f092236 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -5337,6 +5337,52 @@ qemuMonitorJSONParseCPUModelProperty(const char *key,
return 0;
}
+
+/* model_json: {"name": "IvyBridge", "props": {}}
+ */
+static qemuMonitorCPUModelInfoPtr
+qemuMonitorJSONBuildCPUModelInfoFromJSON(virJSONValuePtr cpu_model)
+{
+ virJSONValuePtr cpu_props;
+ qemuMonitorCPUModelInfoPtr machine_model = NULL;
+ qemuMonitorCPUModelInfoPtr model = NULL;
+ char const *cpu_name;
+
+ if (!(cpu_name = virJSONValueObjectGetString(cpu_model, "name"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Parsed JSON reply missing 'name'"));
+ goto cleanup;
+ }
+
+ if (!(cpu_props = virJSONValueObjectGetObject(cpu_model, "props"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Parsed JSON reply missing 'props'"));
+ goto cleanup;
+ }
+
+ if (VIR_ALLOC(machine_model) < 0)
+ goto cleanup;
+
+ if (VIR_STRDUP(machine_model->name, cpu_name) < 0)
+ goto cleanup;
+
+ if (VIR_ALLOC_N(machine_model->props,
+ virJSONValueObjectKeysNumber(cpu_props)) < 0)
+ goto cleanup;
+
+ if (virJSONValueObjectForeachKeyValue(cpu_props,
+ qemuMonitorJSONParseCPUModelProperty,
+ machine_model) < 0)
+ goto cleanup;
+
+ VIR_STEAL_PTR(model, machine_model);
+
+ cleanup:
+ qemuMonitorCPUModelInfoFree(machine_model);
+
+ return model;
+}
+
int
qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
qemuMonitorCPUModelExpansionType type,
@@ -5351,9 +5397,6 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
virJSONValuePtr reply = NULL;
virJSONValuePtr data;
virJSONValuePtr cpu_model;
- virJSONValuePtr cpu_props;
- qemuMonitorCPUModelInfoPtr machine_model = NULL;
- char const *cpu_name;
const char *typeStr = "";
*model_info = NULL;
@@ -5426,38 +5469,12 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
goto retry;
}
- if (!(cpu_name = virJSONValueObjectGetString(cpu_model, "name"))) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("query-cpu-model-expansion reply data was missing
'name'"));
- goto cleanup;
- }
-
- if (!(cpu_props = virJSONValueObjectGetObject(cpu_model, "props"))) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("query-cpu-model-expansion reply data was missing
'props'"));
- goto cleanup;
- }
-
- if (VIR_ALLOC(machine_model) < 0)
- goto cleanup;
-
- if (VIR_STRDUP(machine_model->name, cpu_name) < 0)
- goto cleanup;
-
- if (VIR_ALLOC_N(machine_model->props, virJSONValueObjectKeysNumber(cpu_props))
< 0)
- goto cleanup;
-
- if (virJSONValueObjectForeachKeyValue(cpu_props,
- qemuMonitorJSONParseCPUModelProperty,
- machine_model) < 0)
+ if (!(*model_info = qemuMonitorJSONBuildCPUModelInfoFromJSON(cpu_model)))
goto cleanup;
ret = 0;
- *model_info = machine_model;
- machine_model = NULL;
cleanup:
- qemuMonitorCPUModelInfoFree(machine_model);
virJSONValueFree(cmd);
virJSONValueFree(reply);
virJSONValueFree(model);
--
2.14.1
9:55 p.m.
New subject: [libvirt] [PATCHv3 2/3] qemu_monitor_json: Build Json CPU Model Info
Function qemuMonitorJSONBuildCPUModelInfoToJSON
builds and returns JSON of form
{"model": {"name": "IvyBridge", "props": {}}}
from qemuMonitorCPUModelInfo.
Function qemuMonitorJSONBuildCPUModelInfoToJSON
returns virJsonValuePtr on success and NULL on failure.
---
src/qemu/qemu_monitor_json.c | 50 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 50 insertions(+)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 83f092236..3a28f3865 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -5338,6 +5338,56 @@ qemuMonitorJSONParseCPUModelProperty(const char *key,
}
+/* model_json: {"name": "IvyBridge", "props": {}}
+ */
+static virJSONValuePtr
+qemuMonitorJSONBuildCPUModelInfoToJSON(qemuMonitorCPUModelInfoPtr model)
+{
+ virJSONValuePtr cpu_props = NULL;
+ virJSONValuePtr model_json = NULL;
+ size_t i;
+
+ if (!(cpu_props = virJSONValueNewObject()))
+ goto cleanup;
+
+ for (i = 0; i < model->nprops; i++) {
+ qemuMonitorCPUPropertyPtr prop = &(model->props[i]);
+
+ switch (prop->type) {
+ case QEMU_MONITOR_CPU_PROPERTY_BOOLEAN:
+ if (virJSONValueObjectAppendBoolean(cpu_props, prop->name,
+ prop->value.boolean) < 0)
+ goto cleanup;
+ break;
+
+ case QEMU_MONITOR_CPU_PROPERTY_STRING:
+ if (virJSONValueObjectAppendString(cpu_props, prop->name,
+ prop->value.string) < 0)
+ goto cleanup;
+ break;
+
+ case QEMU_MONITOR_CPU_PROPERTY_NUMBER:
+ if (virJSONValueObjectAppendNumberLong(cpu_props, prop->name,
+ prop->value.number) < 0)
+ goto cleanup;
+ break;
+
+ case QEMU_MONITOR_CPU_PROPERTY_LAST:
+ default:
+ virReportEnumRangeError(qemuMonitorCPUPropertyPtr, prop->type);
+ goto cleanup;
+ }
+ }
+
+ ignore_value(virJSONValueObjectCreate(&model_json, "s:name",
model->name,
+ "a:props", &cpu_props, NULL));
+
+ cleanup:
+ virJSONValueFree(cpu_props);
+ return model_json;
+}
+
+
/* model_json: {"name": "IvyBridge", "props": {}}
*/
static qemuMonitorCPUModelInfoPtr
--
2.14.1
9:55 p.m.
New subject: [libvirt] [PATCHv3 3/3] qemu_monitor: query-cpu-model-baseline QMP command
Function qemuMonitorGetCPUModelBaseline exposed to carry out a QMP
query-cpu-model-baseline transaction with QEMU.
QEMU determines a baseline CPU Model from two input CPU Models to
complete the query-cpu-model-baseline transaction.
---
src/qemu/qemu_monitor.c | 13 +++++++++
src/qemu/qemu_monitor.h | 5 ++++
src/qemu/qemu_monitor_json.c | 68 ++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.h | 7 +++++
4 files changed, 93 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 7b647525b..c1098ff72 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3874,6 +3874,19 @@ qemuMonitorCPUModelInfoCopy(const qemuMonitorCPUModelInfo *orig)
return NULL;
}
+int
+qemuMonitorGetCPUModelBaseline(qemuMonitorPtr mon,
+ qemuMonitorCPUModelInfoPtr model_a,
+ qemuMonitorCPUModelInfoPtr model_b,
+ qemuMonitorCPUModelInfoPtr *model_baseline)
+{
+ VIR_DEBUG("model_a->name=%s", model_a->name);
+ VIR_DEBUG("model_b->name=%s", model_b->name);
+
+ QEMU_CHECK_MONITOR_JSON(mon);
+
+ return qemuMonitorJSONGetCPUModelBaseline(mon, model_a, model_b, model_baseline);
+}
int
qemuMonitorGetCommands(qemuMonitorPtr mon,
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index d04148e56..c7a80ca63 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1079,6 +1079,11 @@ void qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr
model_info);
qemuMonitorCPUModelInfoPtr
qemuMonitorCPUModelInfoCopy(const qemuMonitorCPUModelInfo *orig);
+int qemuMonitorGetCPUModelBaseline(qemuMonitorPtr mon,
+ qemuMonitorCPUModelInfoPtr model_a,
+ qemuMonitorCPUModelInfoPtr model_b,
+ qemuMonitorCPUModelInfoPtr *model_baseline);
+
int qemuMonitorGetCommands(qemuMonitorPtr mon,
char ***commands);
int qemuMonitorGetEvents(qemuMonitorPtr mon,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 3a28f3865..1b8a3af43 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -5533,6 +5533,74 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
}
+/* Note: *model_baseline == NULL && return == 0 if command not supported by QEMU
+ */
+int
+qemuMonitorJSONGetCPUModelBaseline(qemuMonitorPtr mon,
+ qemuMonitorCPUModelInfoPtr model_a,
+ qemuMonitorCPUModelInfoPtr model_b,
+ qemuMonitorCPUModelInfoPtr *model_baseline)
+{
+ int ret = -1;
+ virJSONValuePtr cmd = NULL;
+ virJSONValuePtr reply = NULL;
+ virJSONValuePtr data = NULL;
+ virJSONValuePtr modela = NULL;
+ virJSONValuePtr modelb = NULL;
+ virJSONValuePtr cpu_model = NULL;
+
+ *model_baseline = NULL;
+
+ if (!(modela = qemuMonitorJSONBuildCPUModelInfoToJSON(model_a)))
+ goto cleanup;
+
+ if (!(modelb = qemuMonitorJSONBuildCPUModelInfoToJSON(model_b)))
+ goto cleanup;
+
+ if (!(cmd = qemuMonitorJSONMakeCommand("query-cpu-model-baseline",
+ "a:modela", &modela,
+ "a:modelb", &modelb,
+ NULL)))
+ goto cleanup;
+
+ if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
+ goto cleanup;
+
+ /* Urgh, some QEMU architectures have query-cpu-model-baseline
+ * command but return 'GenericError' with string "Not supported",
+ * instead of simply omitting the command entirely
+ */
+ if (qemuMonitorJSONHasError(reply, "GenericError")) {
+ ret = 0;
+ goto cleanup;
+ }
+
+ if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_OBJECT) < 0)
+ goto cleanup;
+
+ data = virJSONValueObjectGetObject(reply, "return");
+
+ if (!(cpu_model = virJSONValueObjectGetObject(data, "model"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("query-cpu-model-baseline reply data was missing
'model'"));
+ goto cleanup;
+ }
+
+ if (!(*model_baseline = qemuMonitorJSONBuildCPUModelInfoFromJSON(cpu_model)))
+ goto cleanup;
+
+ ret = 0;
+
+ cleanup:
+ virJSONValueFree(cmd);
+ virJSONValueFree(reply);
+ virJSONValueFree(modela);
+ virJSONValueFree(modelb);
+
+ return ret;
+}
+
+
int qemuMonitorJSONGetCommands(qemuMonitorPtr mon,
char ***commands)
{
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 045df4919..aa6f3582e 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -361,6 +361,13 @@ int qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
qemuMonitorCPUModelInfoPtr *model_info)
ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(5);
+int qemuMonitorJSONGetCPUModelBaseline(qemuMonitorPtr mon,
+ qemuMonitorCPUModelInfoPtr model_a,
+ qemuMonitorCPUModelInfoPtr model_b,
+ qemuMonitorCPUModelInfoPtr *model_baseline)
+ ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
+
+
int qemuMonitorJSONGetCommands(qemuMonitorPtr mon,
char ***commands)
ATTRIBUTE_NONNULL(2);
--
2.14.1
8:57 a.m.
On 04/30/2018 10:55 PM, Chris Venteicher wrote:
This is part of resolution of:
https://bugzilla.redhat.com/show_bug.cgi?id=1511999
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
diff to v1:
- Replaced c++ style comments with c style
- qemuMonitorJSONGetCPUModelInfo{ToJSON,FromJSON} use To/From form
- qemuMonitorJSONGetCPUModelInfo{ToJSON,FromJSON} return pointers not integers
- VIR_STEAL_PTR form used
- switch statement uses virReportEnumRangeError
- qemuMonitorJSONHasError(reply, "GenericError") treated as no info, not fatal
error
- virJSONValueFree(cpu_props) during error case
diff to v2:
- qemuMonitorJSONGetCPUModelInfo{ToJSON,FromJSON}
works on {"name": "IvyBridge", "props": {}}
rather than {"model": {"name": "IvyBridge",
"props": {}}}
- additional cleanups and fixes
Chris Venteicher (3):
qemu_monitor_json: Populate CPUModelInfo struct from json
qemu_monitor_json: Build Json CPU Model Info
qemu_monitor: query-cpu-model-baseline QMP command
src/qemu/qemu_monitor.c | 13 ++++
src/qemu/qemu_monitor.h | 5 ++
src/qemu/qemu_monitor_json.c | 179 +++++++++++++++++++++++++++++++++++++------
src/qemu/qemu_monitor_json.h | 7 ++
4 files changed, 182 insertions(+), 22 deletions(-)
Changes look good to me,
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
I have to wait for 4.4.0 to open before pushing, but it would also be
good to know what the plans are for code that will use the new
functions. Do you plan on posting something for 4.4.0?
John
3:17 p.m.
Quoting John Ferlan (2018-05-01 08:57:08)
On 04/30/2018 10:55 PM, Chris Venteicher wrote:
> This is part of resolution of: https://bugzilla.redhat.com/show_bug.cgi?id=1511999
>
> Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
>
> diff to v1:
> - Replaced c++ style comments with c style
> - qemuMonitorJSONGetCPUModelInfo{ToJSON,FromJSON} use To/From form
> - qemuMonitorJSONGetCPUModelInfo{ToJSON,FromJSON} return pointers not integers
> - VIR_STEAL_PTR form used
> - switch statement uses virReportEnumRangeError
> - qemuMonitorJSONHasError(reply, "GenericError") treated as no info, not
fatal error
> - virJSONValueFree(cpu_props) during error case
>
> diff to v2:
> - qemuMonitorJSONGetCPUModelInfo{ToJSON,FromJSON}
> works on {"name": "IvyBridge", "props": {}}
> rather than {"model": {"name": "IvyBridge",
"props": {}}}
> - additional cleanups and fixes
>
> Chris Venteicher (3):
> qemu_monitor_json: Populate CPUModelInfo struct from json
> qemu_monitor_json: Build Json CPU Model Info
> qemu_monitor: query-cpu-model-baseline QMP command
>
> src/qemu/qemu_monitor.c | 13 ++++
> src/qemu/qemu_monitor.h | 5 ++
> src/qemu/qemu_monitor_json.c | 179 +++++++++++++++++++++++++++++++++++++------
> src/qemu/qemu_monitor_json.h | 7 ++
> 4 files changed, 182 insertions(+), 22 deletions(-)
>
Changes look good to me,
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
I have to wait for 4.4.0 to open before pushing, but it would also be
good to know what the plans are for code that will use the new
functions. Do you plan on posting something for 4.4.0?
I plan on posting something yet this week that uses this qemu_monitor interface.
It will likely take some rounds of feedback before the new patch set is ready to
be accepted. Not sure how this maps to 4.4.0 window.
However, I don't think there is much that could happen with the new patch set
that would change the need for the qemu_monitor interface exposed by this patch
set.
Thanks, Chris
John
2:43 a.m.
On Tue, May 01, 2018 at 09:57:08 -0400, John Ferlan wrote:
On 04/30/2018 10:55 PM, Chris Venteicher wrote:
> This is part of resolution of: https://bugzilla.redhat.com/show_bug.cgi?id=1511999
>
> Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
>
> diff to v1:
> - Replaced c++ style comments with c style
> - qemuMonitorJSONGetCPUModelInfo{ToJSON,FromJSON} use To/From form
> - qemuMonitorJSONGetCPUModelInfo{ToJSON,FromJSON} return pointers not integers
> - VIR_STEAL_PTR form used
> - switch statement uses virReportEnumRangeError
> - qemuMonitorJSONHasError(reply, "GenericError") treated as no info, not
fatal error
> - virJSONValueFree(cpu_props) during error case
>
> diff to v2:
> - qemuMonitorJSONGetCPUModelInfo{ToJSON,FromJSON}
> works on {"name": "IvyBridge", "props": {}}
> rather than {"model": {"name": "IvyBridge",
"props": {}}}
> - additional cleanups and fixes
>
> Chris Venteicher (3):
> qemu_monitor_json: Populate CPUModelInfo struct from json
> qemu_monitor_json: Build Json CPU Model Info
> qemu_monitor: query-cpu-model-baseline QMP command
>
> src/qemu/qemu_monitor.c | 13 ++++
> src/qemu/qemu_monitor.h | 5 ++
> src/qemu/qemu_monitor_json.c | 179 +++++++++++++++++++++++++++++++++++++------
> src/qemu/qemu_monitor_json.h | 7 ++
> 4 files changed, 182 insertions(+), 22 deletions(-)
>
Changes look good to me,
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
I have to wait for 4.4.0 to open before pushing, but it would also be
good to know what the plans are for code that will use the new
functions. Do you plan on posting something for 4.4.0?
I don't think pushing this makes a lot of sense without any users.
Without seeing the complete picture from a public APIs down to the
monitor code it's a bit hard to see if the code fits together nicely. We
have two approaches now... Chris makes the monitor APIs work on
CPUModelInfo while Collin's APIs use virCPUDef. We need to see how this
all fits into the public APIs first.
Jirka
2:34 p.m.
On 05/02/2018 03:43 AM, Jiri Denemark wrote:
On Tue, May 01, 2018 at 09:57:08 -0400, John Ferlan wrote:
>
>
> On 04/30/2018 10:55 PM, Chris Venteicher wrote:
>> This is part of resolution of:
https://bugzilla.redhat.com/show_bug.cgi?id=1511999
>>
>> Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
>>
>> diff to v1:
>> - Replaced c++ style comments with c style
>> - qemuMonitorJSONGetCPUModelInfo{ToJSON,FromJSON} use To/From form
>> - qemuMonitorJSONGetCPUModelInfo{ToJSON,FromJSON} return pointers not integers
>> - VIR_STEAL_PTR form used
>> - switch statement uses virReportEnumRangeError
>> - qemuMonitorJSONHasError(reply, "GenericError") treated as no info,
not fatal error
>> - virJSONValueFree(cpu_props) during error case
>>
>> diff to v2:
>> - qemuMonitorJSONGetCPUModelInfo{ToJSON,FromJSON}
>> works on {"name": "IvyBridge", "props": {}}
>> rather than {"model": {"name": "IvyBridge",
"props": {}}}
>> - additional cleanups and fixes
>>
>> Chris Venteicher (3):
>> qemu_monitor_json: Populate CPUModelInfo struct from json
>> qemu_monitor_json: Build Json CPU Model Info
>> qemu_monitor: query-cpu-model-baseline QMP command
>>
>> src/qemu/qemu_monitor.c | 13 ++++
>> src/qemu/qemu_monitor.h | 5 ++
>> src/qemu/qemu_monitor_json.c | 179 +++++++++++++++++++++++++++++++++++++------
>> src/qemu/qemu_monitor_json.h | 7 ++
>> 4 files changed, 182 insertions(+), 22 deletions(-)
>>
>
> Changes look good to me,
>
> Reviewed-by: John Ferlan <jferlan(a)redhat.com>
>
> I have to wait for 4.4.0 to open before pushing, but it would also be
> good to know what the plans are for code that will use the new
> functions. Do you plan on posting something for 4.4.0?
I don't think pushing this makes a lot of sense without any users.
Without seeing the complete picture from a public APIs down to the
monitor code it's a bit hard to see if the code fits together nicely. We
have two approaches now... Chris makes the monitor APIs work on
CPUModelInfo while Collin's APIs use virCPUDef. We need to see how this
all fits into the public APIs first.
Jirka
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
I agree with Jiri. In their current state, I think your patches look fine but
I'll hold off on giving my r-b's until I see how they get used.
Great progress! Looking forward to seeing how it all hooks up.
--
Respectfully,
- Collin Walling
6:59 p.m.
On 05/02/2018 03:34 PM, Collin Walling wrote:
On 05/02/2018 03:43 AM, Jiri Denemark wrote:
> On Tue, May 01, 2018 at 09:57:08 -0400, John Ferlan wrote:
>>
>>
>> On 04/30/2018 10:55 PM, Chris Venteicher wrote:
>>> This is part of resolution of:
https://bugzilla.redhat.com/show_bug.cgi?id=1511999
>>>
>>> Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
>>>
>>> diff to v1:
>>> - Replaced c++ style comments with c style
>>> - qemuMonitorJSONGetCPUModelInfo{ToJSON,FromJSON} use To/From form
>>> - qemuMonitorJSONGetCPUModelInfo{ToJSON,FromJSON} return pointers not
integers
>>> - VIR_STEAL_PTR form used
>>> - switch statement uses virReportEnumRangeError
>>> - qemuMonitorJSONHasError(reply, "GenericError") treated as no
info, not fatal error
>>> - virJSONValueFree(cpu_props) during error case
>>>
>>> diff to v2:
>>> - qemuMonitorJSONGetCPUModelInfo{ToJSON,FromJSON}
>>> works on {"name": "IvyBridge", "props": {}}
>>> rather than {"model": {"name": "IvyBridge",
"props": {}}}
>>> - additional cleanups and fixes
>>>
>>> Chris Venteicher (3):
>>> qemu_monitor_json: Populate CPUModelInfo struct from json
>>> qemu_monitor_json: Build Json CPU Model Info
>>> qemu_monitor: query-cpu-model-baseline QMP command
>>>
>>> src/qemu/qemu_monitor.c | 13 ++++
>>> src/qemu/qemu_monitor.h | 5 ++
>>> src/qemu/qemu_monitor_json.c | 179
+++++++++++++++++++++++++++++++++++++------
>>> src/qemu/qemu_monitor_json.h | 7 ++
>>> 4 files changed, 182 insertions(+), 22 deletions(-)
>>>
>>
>> Changes look good to me,
>>
>> Reviewed-by: John Ferlan <jferlan(a)redhat.com>
>>
>> I have to wait for 4.4.0 to open before pushing, but it would also be
>> good to know what the plans are for code that will use the new
>> functions. Do you plan on posting something for 4.4.0?
>
> I don't think pushing this makes a lot of sense without any users.
> Without seeing the complete picture from a public APIs down to the
> monitor code it's a bit hard to see if the code fits together nicely. We
> have two approaches now... Chris makes the monitor APIs work on
> CPUModelInfo while Collin's APIs use virCPUDef. We need to see how this
> all fits into the public APIs first.
>
> Jirka
>
> --
> libvir-list mailing list
> libvir-list(a)redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>
I agree with Jiri. In their current state, I think your patches look fine but
I'll hold off on giving my r-b's until I see how they get used.
Great progress! Looking forward to seeing how it all hooks up.
Fine by me to wait - yes, it's a bit different order to adjust the
plumbing first... At least if we had to make changes there it wouldn't
affect user/external API's ;-)
Perhaps it's "easier" to process patches in small groups rather than 15,
20, 35, 75, etc. patch bomb series. Just going to have to deal with the
coordination of two people. Hopefully it gets done soon while things are
relatively fresh in mind!
John
2431
days inactive
2432
days old
8 comments
4 participants
participants (4)
-
Chris Venteicher -
Collin Walling -
Jiri Denemark -
John Ferlan