On 01/30/2015 08:21 AM, Peter Krempa wrote:
> Add support to start qemu instance with 'pc-dimm' device. Thanks to the
> refactors we are able to reuse the existing function to determine the
> parameters.
> ---
> src/qemu/qemu_command.c | 114 ++++++++++++++++++++++++++++++++++++++++++++++++
> src/qemu/qemu_domain.c | 18 ++++++--
> src/qemu/qemu_domain.h | 2 +
> tests/qemuxml2xmltest.c | 1 +
> 4 files changed, 132 insertions(+), 3 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 5820fb5..7c31723 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -1156,6 +1156,10 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps)
> if (virAsprintf(&def->tpm->info.alias, "tpm%d", 0) < 0)
> return -1;
> }
> + for (i = 0; i < def->nmems; i++) {
> + if (virAsprintf(&def->mems[i]->info.alias, "dimm%zu", i) < 0)
> + return -1;
> + }
>
> return 0;
> }
> @@ -4748,6 +4752,97 @@ qemuBuildMemoryCellBackendStr(virDomainDefPtr def,
> }
>
>
> +static char *
> +qemuBuildMemoryDimmBackendStr(virDomainMemoryDefPtr mem,
> + virDomainDefPtr def,
> + virQEMUCapsPtr qemuCaps,
> + virQEMUDriverConfigPtr cfg)
> +{
> + virJSONValuePtr props = NULL;
> + char *alias = NULL;
> + const char *backendType;
> + char *ret = NULL;
> +
> + if (!mem->info.alias) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("memory device alias is not assigned"));
> + return NULL;
> + }
> +
> + if (virAsprintf(&alias, "mem%s", mem->info.alias) < 0)
> + goto cleanup;
> +
> + qemuDomainMemoryDeviceAlignSize(mem);
> +
> + if (qemuBuildMemoryBackendStr(mem->size, mem->pagesize,
> + mem->targetNode, mem->sourceNodes, NULL,
> + def, qemuCaps, cfg,
> + &backendType, &props, true) < 0)
> + goto cleanup;
> +
> + ret = qemuBuildObjectCommandlineFromJSON(backendType, alias, props);
> +
> + cleanup:
> + VIR_FREE(alias);
> + virJSONValueFree(props);
> +
> + return ret;
> +}
> +
> +
> +static char *
> +qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem,
> + virQEMUCapsPtr qemuCaps)
> +{
> + virBuffer buf = VIR_BUFFER_INITIALIZER;
> +
> + if (!mem->info.alias) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("missing alias for memory device"));
> + return NULL;
> + }
> +
> + switch ((virDomainMemoryModel) mem->model) {
> + case VIR_DOMAIN_MEMORY_MODEL_ACPI_DIMM:
> + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PC_DIMM)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("this qemu doesn't support the pc-dimm device"));
> + return NULL;
> + }
> +
> + if (mem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ACPI_DIMM &&
> + mem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("only 'acpi-dimm' addresses are supported for the "
> + "pc-dimm device"));
> + return NULL;
> + }
> +
> + virBufferAsprintf(&buf, "pc-dimm,node=%d,memdev=mem%s,id=%s",
> + mem->targetNode, mem->info.alias, mem->info.alias);
> +
> + if (mem->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ACPI_DIMM) {
> + virBufferAsprintf(&buf, ",slot=%d", mem->info.addr.acpiDimm.slot);
> + virBufferAsprintf(&buf, ",base=%llu", mem->info.addr.acpiDimm.base);
> + }
> +
> + break;
> +
> + case VIR_DOMAIN_MEMORY_MODEL_NONE:
> + case VIR_DOMAIN_MEMORY_MODEL_LAST:
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("invalid memory device type"));
> + break;
> +
> + }
> +
> + if (virBufferCheckError(&buf) < 0)
> + return NULL;
> +
> + return virBufferContentAndReset(&buf);
> +}
> +
> +
> char *
> qemuBuildNicStr(virDomainNetDefPtr net,
> const char *prefix,
> @@ -8351,6 +8446,25 @@ qemuBuildCommandLine(virConnectPtr conn,
> if (qemuBuildNumaArgStr(cfg, def, cmd, qemuCaps, nodeset) < 0)
> goto error;
>
Coverity has a FORWARD_NULL complaint... Right above this code there's:
8445 if (def->cpu && def->cpu->ncells)
8446 if (qemuBuildNumaArgStr(cfg, def, cmd, qemuCaps, nodeset) < 0)
8447 goto error;
8448
So there's a chance "def->cpu == NULL"
> + for (i = 0; i < def->nmems; i++) {
> + char *backStr;
> + char *dimmStr;
> +
> + if (!(backStr = qemuBuildMemoryDimmBackendStr(def->mems[i], def,
> + qemuCaps, cfg)))
Because def->cpu is NULL above, Coverity points out that
qemuBuildMemoryDimmBackendStr will call qemuBuildMemoryBackendStr which
deref's def->cpu->cells[guestNode].memAccess
John
> + goto error;
> +
> + if (!(dimmStr = qemuBuildMemoryDeviceStr(def->mems[i], qemuCaps))) {
> + VIR_FREE(backStr);
> + goto error;
> + }
> +
> + virCommandAddArgList(cmd, "-object", backStr, "-device", dimmStr, NULL);
> +
> + VIR_FREE(backStr);
> + VIR_FREE(dimmStr);
> + }
> +
> if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_UUID))
> virCommandAddArgList(cmd, "-uuid", uuid, NULL);
> if (def->virtType == VIR_DOMAIN_VIRT_XEN ||
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list