Hi,
On 10/27/20 2:55 PM, Philippe Mathieu-Daudé wrote:
> Just for consistency, following the example documented since
> commit e3fe3988d7 ("error: Document Error API usage rules"),
> return a boolean value indicating an error is set or not.
> This simplifies a bit nvme_create_queue_pair().
also directly pass errp as the local_err is not requested in our case.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> block/nvme.c | 15 ++++++---------
> 1 file changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/block/nvme.c b/block/nvme.c
> index 74994c442e5..9324f0bfdc4 100644
> --- a/block/nvme.c
> +++ b/block/nvme.c
> @@ -160,7 +160,8 @@ static QemuOptsList runtime_opts = {
> },
> };
>
> -static void nvme_init_queue(BDRVNVMeState *s, NVMeQueue *q,
> +/* Returns true on success, false on failure. */
> +static bool nvme_init_queue(BDRVNVMeState *s, NVMeQueue *q,
> unsigned nentries, size_t entry_bytes, Error **errp)
> {
> size_t bytes;
> @@ -171,13 +172,14 @@ static void nvme_init_queue(BDRVNVMeState *s, NVMeQueue *q,
I personally prefer returning a conventional int instead of bool.
> q->queue = qemu_try_memalign(s->page_size, bytes);
> if (!q->queue) {
> error_setg(errp, "Cannot allocate queue");
> - return;
> + return false;
> }
> memset(q->queue, 0, bytes);
> r = qemu_vfio_dma_map(s->vfio, q->queue, bytes, false, &q->iova);
> if (r) {
> error_setg(errp, "Cannot map queue");
> }
> + return r == 0;
also avoids that kind of conversion and use of !() in the called
> }
>
> static void nvme_free_queue_pair(NVMeQueuePair *q)
> @@ -210,7 +212,6 @@ static NVMeQueuePair *nvme_create_queue_pair(BDRVNVMeState *s,
> Error **errp)
> {
> int i, r;
> - Error *local_err = NULL;
> NVMeQueuePair *q;
> uint64_t prp_list_iova;
>
> @@ -247,16 +248,12 @@ static NVMeQueuePair *nvme_create_queue_pair(BDRVNVMeState *s,
> req->prp_list_iova = prp_list_iova + i * s->page_size;
> }
>
> - nvme_init_queue(s, &q->sq, size, NVME_SQ_ENTRY_BYTES, &local_err);
> - if (local_err) {
> - error_propagate(errp, local_err);
> + if (!nvme_init_queue(s, &q->sq, size, NVME_SQ_ENTRY_BYTES, errp)) {
> goto fail;
> }
> q->sq.doorbell = &s->doorbells[idx * s->doorbell_scale].sq_tail;
>
> - nvme_init_queue(s, &q->cq, size, NVME_CQ_ENTRY_BYTES, &local_err);
> - if (local_err) {
> - error_propagate(errp, local_err);
> + if (!nvme_init_queue(s, &q->cq, size, NVME_CQ_ENTRY_BYTES, errp)) {
> goto fail;
> }
> q->cq.doorbell = &s->doorbells[idx * s->doorbell_scale].cq_head;
>
Thanks
Eric