[libvirt] [RFC PATCH 08/12] qemu: add support for memory devices

John Ferlan jferlan at redhat.com
Wed Feb 4 22:28:00 UTC 2015



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 ||




More information about the libvir-list mailing list