diff mbox series

[18/25] block/nvme: Pass AioContext argument to nvme_add_io_queue()

Message ID 20201027135547.374946-19-philmd@redhat.com (mailing list archive)
State New, archived
Headers show
Series block/nvme: Fix Aarch64 host | expand

Commit Message

Philippe Mathieu-Daudé Oct. 27, 2020, 1:55 p.m. UTC
We want to get ride of the BlockDriverState pointer at some point,
so pass aio_context along.

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

Comments

Auger Eric Oct. 28, 2020, 2:30 p.m. UTC | #1
Hi Philippe,

On 10/27/20 2:55 PM, Philippe Mathieu-Daudé wrote:
> We want to get ride of the BlockDriverState pointer at some point,
s/ride/rid
> so pass aio_context along.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  block/nvme.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/block/nvme.c b/block/nvme.c
> index 68f0c3f7959..a0871fc2a81 100644
> --- a/block/nvme.c
> +++ b/block/nvme.c
> @@ -644,7 +644,9 @@ static void nvme_handle_event(EventNotifier *n)
>      nvme_poll_queues(s);
>  }
>  
> -static bool nvme_add_io_queue(BlockDriverState *bs, Error **errp)
> +/* Returns true on success, false on failure. */
belongs to another patch, still not a big fan of bool ;-)
> +static bool nvme_add_io_queue(BlockDriverState *bs,
> +                              AioContext *aio_context, Error **errp)
>  {
>      BDRVNVMeState *s = bs->opaque;
>      unsigned n = s->queue_count;
> @@ -653,8 +655,7 @@ static bool nvme_add_io_queue(BlockDriverState *bs, Error **errp)
>      unsigned queue_size = NVME_QUEUE_SIZE;
>  
>      assert(n <= UINT16_MAX);
> -    q = nvme_create_queue_pair(s, bdrv_get_aio_context(bs),
> -                               n, queue_size, errp);
> +    q = nvme_create_queue_pair(s, aio_context, n, queue_size, errp);
>      if (!q) {
>          return false;
>      }
> @@ -830,7 +831,7 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace,
>      }
>  
>      /* Set up command queues. */
> -    if (!nvme_add_io_queue(bs, errp)) {
> +    if (!nvme_add_io_queue(bs, aio_context, errp)) {
>          ret = -EIO;
>      }
>  out:
> 
Besides
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Eric
Stefan Hajnoczi Oct. 28, 2020, 3:30 p.m. UTC | #2
On Tue, Oct 27, 2020 at 02:55:40PM +0100, Philippe Mathieu-Daudé wrote:
> We want to get ride of the BlockDriverState pointer at some point,
> so pass aio_context along.

Potential future changes are a weak justification. Someone else might
decide that the aio_context argument is redundant and reverse this
change. Either way works, so this change is somewhat arbitrary at the
moment.

A strong justification would be if the next patch removes the BDS
pointer argument.

Until this change is necessary I would prefer not to include it. That
saves reviewer time, reduces the risk of introducing bugs, etc.
diff mbox series

Patch

diff --git a/block/nvme.c b/block/nvme.c
index 68f0c3f7959..a0871fc2a81 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -644,7 +644,9 @@  static void nvme_handle_event(EventNotifier *n)
     nvme_poll_queues(s);
 }
 
-static bool nvme_add_io_queue(BlockDriverState *bs, Error **errp)
+/* Returns true on success, false on failure. */
+static bool nvme_add_io_queue(BlockDriverState *bs,
+                              AioContext *aio_context, Error **errp)
 {
     BDRVNVMeState *s = bs->opaque;
     unsigned n = s->queue_count;
@@ -653,8 +655,7 @@  static bool nvme_add_io_queue(BlockDriverState *bs, Error **errp)
     unsigned queue_size = NVME_QUEUE_SIZE;
 
     assert(n <= UINT16_MAX);
-    q = nvme_create_queue_pair(s, bdrv_get_aio_context(bs),
-                               n, queue_size, errp);
+    q = nvme_create_queue_pair(s, aio_context, n, queue_size, errp);
     if (!q) {
         return false;
     }
@@ -830,7 +831,7 @@  static int nvme_init(BlockDriverState *bs, const char *device, int namespace,
     }
 
     /* Set up command queues. */
-    if (!nvme_add_io_queue(bs, errp)) {
+    if (!nvme_add_io_queue(bs, aio_context, errp)) {
         ret = -EIO;
     }
 out: