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.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 block/nvme.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/block/nvme.c b/block/nvme.c
index 8b0fd59c6ea..74994c442e5 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -506,9 +506,11 @@ static int nvme_cmd_sync(BlockDriverState *bs, NVMeQueuePair *q,
     return ret;
 }
 
-static void nvme_identify(BlockDriverState *bs, int namespace, Error **errp)
+/* Returns true on success, false on failure. */
+static bool nvme_identify(BlockDriverState *bs, int namespace, Error **errp)
 {
     BDRVNVMeState *s = bs->opaque;
+    bool ret = false;
     union {
         NvmeIdCtrl ctrl;
         NvmeIdNs ns;
@@ -585,10 +587,13 @@ static void nvme_identify(BlockDriverState *bs, int namespace, Error **errp)
         goto out;
     }
 
+    ret = true;
     s->blkshift = lbaf->ds;
 out:
     qemu_vfio_dma_unmap(s->vfio, id);
     qemu_vfree(id);
+
+    return ret;
 }
 
 static bool nvme_poll_queue(NVMeQueuePair *q)
-- 
2.26.2

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.
Then I think the returned value should be used by the caller in this patch

Thanks

Eric
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  block/nvme.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/block/nvme.c b/block/nvme.c
> index 8b0fd59c6ea..74994c442e5 100644
> --- a/block/nvme.c
> +++ b/block/nvme.c
> @@ -506,9 +506,11 @@ static int nvme_cmd_sync(BlockDriverState *bs, NVMeQueuePair *q,
>      return ret;
>  }
>  
> -static void nvme_identify(BlockDriverState *bs, int namespace, Error **errp)
> +/* Returns true on success, false on failure. */
> +static bool nvme_identify(BlockDriverState *bs, int namespace, Error **errp)
>  {
>      BDRVNVMeState *s = bs->opaque;
> +    bool ret = false;>      union {
>          NvmeIdCtrl ctrl;
>          NvmeIdNs ns;
> @@ -585,10 +587,13 @@ static void nvme_identify(BlockDriverState *bs, int namespace, Error **errp)
>          goto out;
>      }
>  
> +    ret = true;
>      s->blkshift = lbaf->ds;
>  out:
>      qemu_vfio_dma_unmap(s->vfio, id);
>      qemu_vfree(id);
> +
> +    return ret;
>  }
>  
>  static bool nvme_poll_queue(NVMeQueuePair *q)
> 

On Wed, Oct 28, 2020 at 12:03:00PM +0100, Auger Eric wrote:
> 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.
> Then I think the returned value should be used by the caller in this patch

Yes, please.

Stefan