| Message ID | 20201027135547.374946-19-philmd@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | block/nvme: Fix Aarch64 host | expand |
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
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 --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:
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(-)