Allow case where props not present in JSON for CPUModelInfo. Check for NULL input. Update comments to show JSON examples for more typical S390x usecase. --- src/qemu/qemu_monitor_json.c | 34 +++++++++++++++------------------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index cb17cf53bc..92db267353 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5358,7 +5358,7 @@ qemuMonitorJSONParseCPUModelProperty(const char *key, } -/* model_json: {"name": "IvyBridge", "props": {}} +/* model_json: {"name": "z13-base", "props": {}} */ static virJSONValuePtr qemuMonitorJSONBuildCPUModelInfoToJSON(qemuMonitorCPUModelInfoPtr model) @@ -5367,6 +5367,9 @@ qemuMonitorJSONBuildCPUModelInfoToJSON(qemuMonitorCPUModelInfoPtr model) virJSONValuePtr model_json = NULL; size_t i; + if (!model) + goto cleanup; + if (!(cpu_props = virJSONValueNewObject())) goto cleanup; @@ -5408,7 +5411,7 @@ qemuMonitorJSONBuildCPUModelInfoToJSON(qemuMonitorCPUModelInfoPtr model) } -/* model_json: {"name": "IvyBridge", "props": {}} +/* model_json: {"name": "z13-base", "props": {}} */ static qemuMonitorCPUModelInfoPtr qemuMonitorJSONBuildCPUModelInfoFromJSON(virJSONValuePtr cpu_model) @@ -5424,26 +5427,22 @@ qemuMonitorJSONBuildCPUModelInfoFromJSON(virJSONValuePtr cpu_model) 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 ((cpu_props = virJSONValueObjectGetObject(cpu_model, "props"))) { + if (VIR_ALLOC_N(machine_model->props, + virJSONValueObjectKeysNumber(cpu_props)) < 0) + goto cleanup; - if (virJSONValueObjectForeachKeyValue(cpu_props, - qemuMonitorJSONParseCPUModelProperty, - machine_model) < 0) - goto cleanup; + if (virJSONValueObjectForeachKeyValue(cpu_props, + qemuMonitorJSONParseCPUModelProperty, + machine_model) < 0) + goto cleanup; + } VIR_STEAL_PTR(model, machine_model); @@ -5586,11 +5585,8 @@ qemuMonitorJSONGetCPUModelBaseline(qemuMonitorPtr mon, 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")) { + /* QEMU does not support query-cpu-model-baseline or cpu model */ ret = 0; goto cleanup; } -- 2.14.1

Functions converting directly between virsh XML and QMEU QMP forms of CPUModelInfo. --- src/qemu/qemu_capabilities.c | 159 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 159 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 0265d83028..afce3eb2b7 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -3341,6 +3341,165 @@ virQEMUCapsLoadCache(virArch hostArch, } +/* <cpu> + * <arch>s390x</arch> + * <model>z13-base</model> + * <feature name='xxx'/> + * ... + * <feature name='yyy'/> + * </cpu> + * + * returns 0, (*model), Success + * returns 0, !(*model), Content not found + * returns -1, !(*model), Failure + */ +static int +virQEMUCapsCPUModelInfoFromXML(xmlXPathContextPtr ctxt, + const char *xpath, + qemuMonitorCPUModelInfoPtr *model) +{ + size_t i; + int ret = -1; + qemuMonitorCPUModelInfoPtr cpuModel = NULL; + xmlNodePtr *nodes = NULL; + xmlNodePtr oldnode = ctxt->node; + + VIR_DEBUG("xpath =%s", NULLSTR(xpath)); + + *model = NULL; + + /* optional xpath query for cpu node with nonerror exit on miss */ + if (xpath && !(ctxt->node = virXPathNode(xpath, ctxt))) { + ret = 0; + goto cleanup; + } + + if (!virXMLNodeNameEqual(ctxt->node, "cpu")) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("XML does not contain expected 'cpu' element")); + goto cleanup; + } + + if (VIR_ALLOC(cpuModel) < 0) + goto cleanup; + + if (!(cpuModel->name = virXPathString("string(./model[1])", ctxt))) { + virReportError(VIR_ERR_XML_ERROR, "%s", _("Missing CPU model name")); + goto cleanup; + } + + cpuModel->nprops = virXPathNodeSet("./feature", ctxt, &nodes); + + if (VIR_ALLOC_N(cpuModel->props, cpuModel->nprops) < 0) + goto cleanup; + + for (i = 0; i < cpuModel->nprops; i++) { + qemuMonitorCPUPropertyPtr prop = &(cpuModel->props[i]); + + prop->type = QEMU_MONITOR_CPU_PROPERTY_BOOLEAN; + prop->value.boolean = true; + prop->migratable = VIR_TRISTATE_BOOL_ABSENT; + + if (!(prop->name = virXMLPropString(nodes[i], "name"))) { + virReportError(VIR_ERR_XML_ERROR, "%s", _("Invalid CPU feature name")); + goto cleanup; + } + } + + *model = qemuMonitorCPUModelInfoCopy(cpuModel); + + ret = 0; + + VIR_DEBUG("model->name = %s", NULLSTR((*model)->name)); + + cleanup: + ctxt->node = oldnode; + VIR_FREE(nodes); + qemuMonitorCPUModelInfoFree(cpuModel); + return ret; +} + + +/* + * returns 0, (*model), Success + * returns 0, !(*model), Content not found + * returns -1, !(*model), Failure + */ +static int +virQEMUCapsLoadCPUModelInfoFromXMLString(const char *xml, + const char *xpath, + qemuMonitorCPUModelInfoPtr *model) +{ + int ret = -1; + + xmlDocPtr doc = NULL; + xmlXPathContextPtr ctxt = NULL; + + *model = NULL; + + VIR_DEBUG("input = %s", NULLSTR(xml)); + + if (!(doc = virXMLParseStringCtxt(xml, _("(CPU_definition)"), &ctxt))) { + virReportError(VIR_ERR_INVALID_ARG, "%s", _("missing CPU definition")); + goto cleanup; + } + + if (virQEMUCapsCPUModelInfoFromXML(ctxt, xpath, model) < 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("XML does not contain expected 'cpu' element")); + goto cleanup; + } + + ret = 0; + + cleanup: + xmlXPathFreeContext(ctxt); + xmlFreeDoc(doc); + + return ret; +} + + +/* <cpu match='exact'> + * <model>z13-base</model> + * <feature policy='require' name='xxx'/> + * <feature policy='require' name='yyy'/> + * </cpu> + */ +static void +virQEMUCapsCPUModelInfoToXML(qemuMonitorCPUModelInfoPtr model, virBufferPtr buf) +{ + size_t i; + + if (!model) + return; + + virBufferAddLit(buf, "<cpu match='exact'>\n"); + + virBufferAdjustIndent(buf, 2); + + virBufferAsprintf(buf, "<model>%s</model>\n", model->name); + + for (i = 0; i < model->nprops; i++) { + qemuMonitorCPUPropertyPtr prop = &(model->props[i]); + + virBufferAsprintf(buf, "<feature policy='require' name='%s'/>\n", + prop->name); + } + + virBufferAdjustIndent(buf, -2); + + virBufferAddLit(buf, "</cpu>\n"); +} + + +/* + * <hostCPU type='%s' model='%s' migratability='%s'> + * <property name='%s' type='%s' value='%s'/> + * ... + * <property name='%s' type='%s' value='%s'/> + * </hostCPU> + */ static void virQEMUCapsFormatHostCPUModelInfo(virQEMUCapsPtr qemuCaps, virBufferPtr buf, -- 2.14.1

Start and connect to QEMU so QMP commands can be performed. Isolates code for starting QEMU and establishing Monitor connections from code for obtaining capabilities so that arbitrary QMP commands can be exchanged with QEMU. --- src/qemu/qemu_capabilities.c | 59 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index afce3eb2b7..097985cbe7 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4303,6 +4303,65 @@ virQEMUCapsInitQMPCommandRun(virQEMUCapsInitQMPCommandPtr cmd, } +/* Start and connect to QEMU so QMP commands can be performed. + */ +static virQEMUCapsInitQMPCommandPtr +virQEMUCapsSpinUpQemu(const char *exec, + const char *libDir, uid_t runUid, gid_t runGid, + bool forceTCG) +{ + virQEMUCapsInitQMPCommandPtr cmd = NULL; + virQEMUCapsInitQMPCommandPtr rtn_cmd = NULL; + char *binary = NULL; + + if (exec) { + if (VIR_STRDUP(binary, exec) < 0) + goto cleanup; + } else { + /* Check for existence of base emulator, or alternate base + * which can be used with magic cpu choice + */ + virArch arch = virArchFromHost(); + binary = virQEMUCapsFindBinaryForArch(arch, arch); + } + + VIR_DEBUG("binary=%s", binary); + + /* Make sure the binary we are about to try exec'ing exists. + * Technically we could catch the exec() failure, but that's + * in a sub-process so it's hard to feed back a useful error. + */ + if (!virFileIsExecutable(binary)) { + virReportSystemError(errno, _("QEMU binary %s is not executable"), + binary); + goto cleanup; + } + + if (!(cmd = virQEMUCapsInitQMPCommandNew(binary, libDir, + runUid, runGid, NULL))) + goto cleanup; + + if ((virQEMUCapsInitQMPCommandRun(cmd, forceTCG)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Error starting QEMU")); + goto cleanup; + } + + if (!(cmd->mon)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Error connecting to QEMU")); + goto cleanup; + } + + VIR_STEAL_PTR(rtn_cmd, cmd); + + cleanup: + virQEMUCapsInitQMPCommandFree(cmd); + VIR_FREE(binary); + + return rtn_cmd; +} + + static int virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps, const char *libDir, -- 2.14.1

Spinup QEMU instance and complete QMP query-cpu-model-baseline exchanges to determine CPUModel Baseline. query-cpu-model-baseline only compares two CPUModels so multiple exchanges are needed to evaluate more than two CPUModels. --- src/qemu/qemu_capabilities.c | 132 +++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_capabilities.h | 9 +++ 2 files changed, 141 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 097985cbe7..9ffbf91a90 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -5122,3 +5122,135 @@ virQEMUCapsSetMicrocodeVersion(virQEMUCapsPtr qemuCaps, { qemuCaps->microcodeVersion = microcodeVersion; } + + +/* in: + * <host> + * <cpu> + * <arch>s390x</arch> + * <model>z13-base</model> + * <feature name='xxx'/> + * ... + * <feature name='yyy'/> + * </cpu> + * <cpu> + * <arch>s390x</arch> + * <model>z114-base</model> + * <feature name='xxx'/> + * ... + * <feature name='yyy'/> + * </cpu> + * </host> + * + * out: + * <cpu match='exact'> + * <model>z13-base</model> + * <feature policy='require' name='xxx'/> + * <feature policy='require' name='yyy'/> + * </cpu> + * + * (ret==0) && (baseline.len == 0) if baseline not supported by QEMU + */ +int +virQEMUCapsQMPBaselineCPUModel(char *exec, + const char *libDir, + uid_t runUid, + gid_t runGid, + const char **xmlCPUs, + unsigned int ncpus, + bool migratable, + virBufferPtr baseline) +{ + qemuMonitorCPUModelInfoPtr model_baseline = NULL; + qemuMonitorCPUModelInfoPtr new_model_baseline = NULL; + qemuMonitorCPUModelInfoPtr model_b = NULL; + virQEMUCapsInitQMPCommandPtr cmd = NULL; + virBuffer buf = VIR_BUFFER_INITIALIZER; + bool forceTCG = false; + int ret = -1; + size_t i; + + VIR_DEBUG("ncpus=%i, migratable=%i", ncpus, migratable); + + if (!(cmd = virQEMUCapsSpinUpQemu(exec, libDir, runUid, runGid, forceTCG))) + goto cleanup; + + if (!(cmd->mon)) { + /* Not connected to QEMU (monitor) */ + VIR_DEBUG("QEMU failed to initialize error=%s", virGetLastErrorMessage()); + goto cleanup; + } + + /* QEMU requires qmp_capabilities exchange first */ + if (qemuMonitorSetCapabilities(cmd->mon) < 0) { + VIR_DEBUG("Failed to set monitor capabilities %s", + virGetLastErrorMessage()); + goto cleanup; + } + + if (!xmlCPUs && ncpus != 0) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("nonzero ncpus doesn't match with NULL xmlCPUs")); + goto cleanup; + } + + if (ncpus < 2) { + virReportError(VIR_ERR_INVALID_ARG, "%s", _("baseline < 2 CPUs ")); + goto cleanup; + } + + if (virQEMUCapsLoadCPUModelInfoFromXMLString(xmlCPUs[0], "//cpu", + &model_baseline) < 0) + goto cleanup; + + VIR_DEBUG("xmlCPUs[0] = %s", xmlCPUs[0]); + + for (i = 1; i < ncpus; i++) { + + VIR_DEBUG("xmlCPUs[%lu] = %s", i, xmlCPUs[i]); + + if (virQEMUCapsLoadCPUModelInfoFromXMLString(xmlCPUs[i], "//cpu", &model_b) < 0) + goto cleanup; + + if (!model_baseline || !model_b) { + virReportError(VIR_ERR_INVALID_ARG, "%s", _("xmlCPU without content")); + goto cleanup; + } + + if (qemuMonitorGetCPUModelBaseline(cmd->mon, model_baseline, + model_b, &new_model_baseline) < 0) + goto cleanup; + + if (!new_model_baseline) { + VIR_DEBUG("QEMU didn't support baseline"); + ret = 0; + goto cleanup; + } + + qemuMonitorCPUModelInfoFree(model_baseline); + qemuMonitorCPUModelInfoFree(model_b); + + model_b = NULL; + + model_baseline = new_model_baseline; + } + + VIR_DEBUG("model_baseline->name = %s", NULLSTR(model_baseline->name)); + + virQEMUCapsCPUModelInfoToXML(model_baseline, &buf); + + if (virBufferUse(&buf)) + virBufferAddBuffer(baseline, &buf); + + ret = 0; + + cleanup: + virQEMUCapsInitQMPCommandFree(cmd); + + qemuMonitorCPUModelInfoFree(model_baseline); + qemuMonitorCPUModelInfoFree(model_b); + + VIR_DEBUG("baseline = %s", virBufferCurrentContent(baseline)); + + return ret; +} diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 8da18a8063..2d3e8ce2ad 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -536,6 +536,15 @@ int virQEMUCapsGetMachineTypesCaps(virQEMUCapsPtr qemuCaps, void virQEMUCapsFilterByMachineType(virQEMUCapsPtr qemuCaps, const char *machineType); +int virQEMUCapsQMPBaselineCPUModel(char *exec, + const char *libDir, + uid_t runUid, + gid_t runGid, + const char **xmlCPUs, + unsigned int ncpus, + bool migratable, + virBufferPtr baseline); + virFileCachePtr virQEMUCapsCacheNew(const char *libDir, const char *cacheDir, uid_t uid, -- 2.14.1

S390 uses qemu-kvm in /usr/libexec. --- src/qemu/qemu_capabilities.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 9ffbf91a90..ac7569679c 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -687,6 +687,11 @@ virQEMUCapsFindBinaryForArch(virArch hostarch, goto out; } + /* Fourth attempt: try 'qemu-kvm' */ + if ((ret = virQEMUCapsFindBinary("%s", "/usr/libexec/qemu-kvm")) != NULL) + goto out; + + out: return ret; } -- 2.14.1

On 05/17/2018 04:41 PM, Jiri Denemark wrote: This is going to sound silly since the string "redhat" is found everywhere in these emails, but do you think it's worthwhile to also add the ability to probe the other native binaries that are used by other distros? For example (and I believe is why you mention "this is handled elsewhere") virQEMUCapsInitGuest (src/qemu/qemu_capabilities.c) will check for the different "kvmbins" for RHEL, Fedora, and Debian/Ubuntu. A little bit of work would allow us to reuse this to get the appropriate qemu caps for the other distros. A "virQEMUCapsFindNativeBinary" or something like that comes to mind. Just a thought. -- Respectfully, - Collin Walling

Allow QEMU process to be started without requirement for caller to maintain handle to qmperr pointer. The handle to qmperr pointer can be stored in QMPCommand structure (new way) stored in calling function's stack (original way). --- src/qemu/qemu_capabilities.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index ac7569679c..431378cc02 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4110,6 +4110,7 @@ struct _virQEMUCapsInitQMPCommand { uid_t runUid; gid_t runGid; char **qmperr; + char *qmperr_internal; char *monarg; char *monpath; char *pidfile; @@ -4187,7 +4188,11 @@ virQEMUCapsInitQMPCommandNew(char *binary, cmd->runUid = runUid; cmd->runGid = runGid; - cmd->qmperr = qmperr; + + if (qmperr) + cmd->qmperr = qmperr; /* external storage */ + else + cmd->qmperr = &cmd->qmperr_internal; /* cmd internal storage */ /* the ".sock" sufix is important to avoid a possible clash with a qemu * domain called "capabilities" -- 2.14.1

virsh cpu-baseline determines baseline from QEMU for some architectures and libvirt for others. Skip the QEMU attempt to save time on architectures QEMU baseline does not support. --- src/qemu/qemu_driver.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 83fc191085..6096d46ab5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13166,6 +13166,15 @@ qemuConnectBaselineCPU(virConnectPtr conn ATTRIBUTE_UNUSED, virCPUDefPtr baseline = NULL; virCPUDefPtr cpu = NULL; char *cpustr = NULL; + virBuffer baseline_xml = VIR_BUFFER_INITIALIZER; + virArch arch = virArchFromHost(); + + VIR_DEBUG("ncpus=%i, flags=0x%x", ncpus, flags); + + virQEMUDriverPtr driver = conn->privateData; + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + + bool migratable = !!(flags & VIR_CONNECT_BASELINE_CPU_MIGRATABLE); virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES | VIR_CONNECT_BASELINE_CPU_MIGRATABLE, NULL); @@ -13173,6 +13182,23 @@ qemuConnectBaselineCPU(virConnectPtr conn ATTRIBUTE_UNUSED, if (virConnectBaselineCPUEnsureACL(conn) < 0) goto cleanup; + + if (!ARCH_IS_S390(arch)) + goto libvirt_method; /* Avoid starting QEMU */ + + /* Try QEMU method + */ + if (virQEMUCapsQMPBaselineCPUModel(NULL, + cfg->libDir, cfg->user, cfg->group, + xmlCPUs, ncpus, migratable, &baseline_xml) < 0) { + /* Content Error */ + goto cleanup; + } + + if ((cpustr = virBufferContentAndReset(&baseline_xml))) + goto done; + + libvirt_method: if (!(cpus = virCPUDefListParse(xmlCPUs, ncpus, VIR_CPU_TYPE_HOST))) goto cleanup; @@ -13199,6 +13225,9 @@ qemuConnectBaselineCPU(virConnectPtr conn ATTRIBUTE_UNUSED, virCPUDefFree(baseline); virCPUDefFree(cpu); + done: + VIR_DEBUG("qemu cpustr = %s", NULLSTR(cpustr)); + return cpustr; } -- 2.14.1