On Tue, Feb 21, 2017 at 23:11:54 -0500, John Ferlan wrote:
On 02/15/2017 11:44 AM, Jiri Denemark wrote:
The static CPU model expansion is designed to return only canonical names of all CPU properties. TO maintain backward compatibility libvirt
s/TO/To
is stuck with different spelling of some of the features, which is only returned by the full expansion. But in addition to returned all spelling
s/returned/returning
variants for all properties the full expansion will contain properties which are not guaranteed to be migration compatible. We need to combine both expansions. First we need to call the static expansion to limit the result to migratable properties. Then we can use the result of the static expansion as an input to the full expansion to get both canonical names and their aliases.
...
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index dd7907482..0454571c1 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5026,7 +5026,7 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, qemuMonitorCPUModelInfoPtr *model_info) { int ret = -1; - virJSONValuePtr model; + virJSONValuePtr model = NULL; virJSONValuePtr cmd = NULL; virJSONValuePtr reply = NULL; virJSONValuePtr data; @@ -5038,16 +5038,24 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
*model_info = NULL;
- if (!(model = virJSONValueNewObject())) - goto cleanup; + retry: + if (!model) { + if (!(model = virJSONValueNewObject())) + goto cleanup;
- if (virJSONValueObjectAppendString(model, "name", model_name) < 0) - goto cleanup; + if (virJSONValueObjectAppendString(model, "name", model_name) < 0) + goto cleanup; + }
switch (type) { case QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC: + case QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_FULL: typeStr = "static"; break; + + case QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL: + typeStr = "full"; + break; }
if (!(cmd = qemuMonitorJSONMakeCommand("query-cpu-model-expansion", @@ -5084,6 +5092,16 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, goto cleanup; }
+ if (type == QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_FULL) { + if (!(model = virJSONValueCopy(cpu_model))) + goto cleanup; + + virJSONValueFree(cmd); + virJSONValueFree(reply); + type = QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL; + goto retry;
When you get here, model must be set.. The retry label tests for not set, which cannot be true - so why would the retry label be on the switch statement? If it did move, then the move of the AppendString inside the "if" wouldn't be necessary.
Sure, no idea what I was thinking about :-)
+ } +
This just seems odd - it's not really a retry, it's like piling on. To me retry is like trying again because something failed. In this case you get static, but then add on the full afterwards. I don't have a better suggestion for a label name.
I used "retry" since it's a common name for backward jumps. Anyway I think a small comment explaining why we are jumping back will help... Jirka