Skip to content

Commit

Permalink
hw/nvme: clean up confusing use of errp/local_err
Browse files Browse the repository at this point in the history
Remove an unnecessary local Error value in nvme_realize(). In the
process, change nvme_check_constraints() to return a bool.

Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
(cherry picked from commit 784fd35)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
(Mjt: needed for v8.2.0-2319-gfa905f65c5
 "hw/nvme: add machine compatibility parameter to enable msix exclusive bar")
  • Loading branch information
birkelund authored and Michael Tokarev committed Mar 13, 2024
1 parent 870a6e2 commit ce08ab6
Showing 1 changed file with 23 additions and 25 deletions.
48 changes: 23 additions & 25 deletions hw/nvme/ctrl.c
Original file line number Diff line number Diff line change
Expand Up @@ -6980,7 +6980,7 @@ static const MemoryRegionOps nvme_cmb_ops = {
},
};

static void nvme_check_constraints(NvmeCtrl *n, Error **errp)
static bool nvme_check_params(NvmeCtrl *n, Error **errp)
{
NvmeParams *params = &n->params;

Expand All @@ -6994,38 +6994,38 @@ static void nvme_check_constraints(NvmeCtrl *n, Error **errp)
if (n->namespace.blkconf.blk && n->subsys) {
error_setg(errp, "subsystem support is unavailable with legacy "
"namespace ('drive' property)");
return;
return false;
}

if (params->max_ioqpairs < 1 ||
params->max_ioqpairs > NVME_MAX_IOQPAIRS) {
error_setg(errp, "max_ioqpairs must be between 1 and %d",
NVME_MAX_IOQPAIRS);
return;
return false;
}

if (params->msix_qsize < 1 ||
params->msix_qsize > PCI_MSIX_FLAGS_QSIZE + 1) {
error_setg(errp, "msix_qsize must be between 1 and %d",
PCI_MSIX_FLAGS_QSIZE + 1);
return;
return false;
}

if (!params->serial) {
error_setg(errp, "serial property not set");
return;
return false;
}

if (n->pmr.dev) {
if (host_memory_backend_is_mapped(n->pmr.dev)) {
error_setg(errp, "can't use already busy memdev: %s",
object_get_canonical_path_component(OBJECT(n->pmr.dev)));
return;
return false;
}

if (!is_power_of_2(n->pmr.dev->size)) {
error_setg(errp, "pmr backend size needs to be power of 2 in size");
return;
return false;
}

host_memory_backend_set_mapped(n->pmr.dev, true);
Expand All @@ -7034,72 +7034,72 @@ static void nvme_check_constraints(NvmeCtrl *n, Error **errp)
if (n->params.zasl > n->params.mdts) {
error_setg(errp, "zoned.zasl (Zone Append Size Limit) must be less "
"than or equal to mdts (Maximum Data Transfer Size)");
return;
return false;
}

if (!n->params.vsl) {
error_setg(errp, "vsl must be non-zero");
return;
return false;
}

if (params->sriov_max_vfs) {
if (!n->subsys) {
error_setg(errp, "subsystem is required for the use of SR-IOV");
return;
return false;
}

if (params->sriov_max_vfs > NVME_MAX_VFS) {
error_setg(errp, "sriov_max_vfs must be between 0 and %d",
NVME_MAX_VFS);
return;
return false;
}

if (params->cmb_size_mb) {
error_setg(errp, "CMB is not supported with SR-IOV");
return;
return false;
}

if (n->pmr.dev) {
error_setg(errp, "PMR is not supported with SR-IOV");
return;
return false;
}

if (!params->sriov_vq_flexible || !params->sriov_vi_flexible) {
error_setg(errp, "both sriov_vq_flexible and sriov_vi_flexible"
" must be set for the use of SR-IOV");
return;
return false;
}

if (params->sriov_vq_flexible < params->sriov_max_vfs * 2) {
error_setg(errp, "sriov_vq_flexible must be greater than or equal"
" to %d (sriov_max_vfs * 2)", params->sriov_max_vfs * 2);
return;
return false;
}

if (params->max_ioqpairs < params->sriov_vq_flexible + 2) {
error_setg(errp, "(max_ioqpairs - sriov_vq_flexible) must be"
" greater than or equal to 2");
return;
return false;
}

if (params->sriov_vi_flexible < params->sriov_max_vfs) {
error_setg(errp, "sriov_vi_flexible must be greater than or equal"
" to %d (sriov_max_vfs)", params->sriov_max_vfs);
return;
return false;
}

if (params->msix_qsize < params->sriov_vi_flexible + 1) {
error_setg(errp, "(msix_qsize - sriov_vi_flexible) must be"
" greater than or equal to 1");
return;
return false;
}

if (params->sriov_max_vi_per_vf &&
(params->sriov_max_vi_per_vf - 1) % NVME_VF_RES_GRANULARITY) {
error_setg(errp, "sriov_max_vi_per_vf must meet:"
" (sriov_max_vi_per_vf - 1) %% %d == 0 and"
" sriov_max_vi_per_vf >= 1", NVME_VF_RES_GRANULARITY);
return;
return false;
}

if (params->sriov_max_vq_per_vf &&
Expand All @@ -7108,9 +7108,11 @@ static void nvme_check_constraints(NvmeCtrl *n, Error **errp)
error_setg(errp, "sriov_max_vq_per_vf must meet:"
" (sriov_max_vq_per_vf - 1) %% %d == 0 and"
" sriov_max_vq_per_vf >= 2", NVME_VF_RES_GRANULARITY);
return;
return false;
}
}

return true;
}

static void nvme_init_state(NvmeCtrl *n)
Expand Down Expand Up @@ -7509,7 +7511,6 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
{
NvmeCtrl *n = NVME(pci_dev);
NvmeNamespace *ns;
Error *local_err = NULL;
NvmeCtrl *pn = NVME(pcie_sriov_get_pf(pci_dev));

if (pci_is_vf(pci_dev)) {
Expand All @@ -7521,17 +7522,14 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
n->subsys = pn->subsys;
}

nvme_check_constraints(n, &local_err);
if (local_err) {
error_propagate(errp, local_err);
if (!nvme_check_params(n, errp)) {
return;
}

qbus_init(&n->bus, sizeof(NvmeBus), TYPE_NVME_BUS,
&pci_dev->qdev, n->parent_obj.qdev.id);

if (nvme_init_subsys(n, errp)) {
error_propagate(errp, local_err);
return;
}
nvme_init_state(n);
Expand Down

0 comments on commit ce08ab6

Please sign in to comment.