diff mbox series

[2/9] block/nvme: Have nvme_create_queue_pair() report errors consistently

Message ID 20210824141142.1165291-3-philmd@redhat.com
State New
Headers show
Series block/nvme: Rework error reporting | expand

Commit Message

Philippe Mathieu-Daudé Aug. 24, 2021, 2:11 p.m. UTC
nvme_create_queue_pair() does not return a boolean value (indicating
eventual error) but a pointer, and is inconsistent in how it fills the
error handler. To fulfill callers expectations, always set an error
message on failure.

Reported-by: Auger Eric <eric.auger@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 block/nvme.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Klaus Jensen Aug. 25, 2021, 11:24 a.m. UTC | #1
On Aug 24 16:11, Philippe Mathieu-Daudé wrote:
> nvme_create_queue_pair() does not return a boolean value (indicating
> eventual error) but a pointer, and is inconsistent in how it fills the
> error handler. To fulfill callers expectations, always set an error
> message on failure.
> 
> Reported-by: Auger Eric <eric.auger@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  block/nvme.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/block/nvme.c b/block/nvme.c
> index e8dbbc23177..6642c104aa4 100644
> --- a/block/nvme.c
> +++ b/block/nvme.c
> @@ -220,6 +220,7 @@ static NVMeQueuePair *nvme_create_queue_pair(BDRVNVMeState *s,
>  
>      q = g_try_new0(NVMeQueuePair, 1);
>      if (!q) {
> +        error_setg(errp, "Cannot allocate queue pair");
>          return NULL;
>      }
>      trace_nvme_create_queue_pair(idx, q, size, aio_context,
> @@ -228,6 +229,7 @@ static NVMeQueuePair *nvme_create_queue_pair(BDRVNVMeState *s,
>                            qemu_real_host_page_size);
>      q->prp_list_pages = qemu_try_memalign(qemu_real_host_page_size, bytes);
>      if (!q->prp_list_pages) {
> +        error_setg(errp, "Cannot allocate PRP page list");
>          goto fail;
>      }
>      memset(q->prp_list_pages, 0, bytes);
> -- 
> 2.31.1
> 

Looks like

    r = qemu_vfio_dma_map(s->vfio, q->prp_list_pages, bytes,
                          false, &prp_list_iova);
    if (r) {
        goto fail;
    }

should set an error message as well.
diff mbox series

Patch

diff --git a/block/nvme.c b/block/nvme.c
index e8dbbc23177..6642c104aa4 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -220,6 +220,7 @@  static NVMeQueuePair *nvme_create_queue_pair(BDRVNVMeState *s,
 
     q = g_try_new0(NVMeQueuePair, 1);
     if (!q) {
+        error_setg(errp, "Cannot allocate queue pair");
         return NULL;
     }
     trace_nvme_create_queue_pair(idx, q, size, aio_context,
@@ -228,6 +229,7 @@  static NVMeQueuePair *nvme_create_queue_pair(BDRVNVMeState *s,
                           qemu_real_host_page_size);
     q->prp_list_pages = qemu_try_memalign(qemu_real_host_page_size, bytes);
     if (!q->prp_list_pages) {
+        error_setg(errp, "Cannot allocate PRP page list");
         goto fail;
     }
     memset(q->prp_list_pages, 0, bytes);