Re: [PATCH v5 12/15] block/nvme: Replace BDRV_POLL_WHILE by AIO_WAIT_WHILE

[Stefano Garzarella]

On Fri, Aug 21, 2020 at 03:15:58PM +0200, Philippe Mathieu-Daudé wrote:
> On 8/21/20 12:15 PM, Stefano Garzarella wrote:
> > On Thu, Aug 20, 2020 at 06:58:58PM +0200, Philippe Mathieu-Daudé wrote:
> >> BDRV_POLL_WHILE() is defined as:
> >>
> >>   #define BDRV_POLL_WHILE(bs, cond) ({          \
> >>       BlockDriverState *bs_ = (bs);             \
> >>       AIO_WAIT_WHILE(bdrv_get_aio_context(bs_), \
> >>                      cond); })
> >>
> >> As we will remove the BlockDriverState use in the next commit,
> >> start by using the exploded version of BDRV_POLL_WHILE().
> >>
> >> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> >> ---
> >>  block/nvme.c | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/block/nvme.c b/block/nvme.c
> >> index 5b69fc75a60..456fe61f5ea 100644
> >> --- a/block/nvme.c
> >> +++ b/block/nvme.c
> >> @@ -493,6 +493,7 @@ static void nvme_cmd_sync_cb(void *opaque, int ret)
> >>  static int nvme_cmd_sync(BlockDriverState *bs, NVMeQueuePair *q,
> >>                           NvmeCmd *cmd)
> >>  {
> >> +    AioContext *aio_context = bdrv_get_aio_context(bs);
> >>      NVMeRequest *req;
> >>      int ret = -EINPROGRESS;
> >>      req = nvme_get_free_req(q);
> >> @@ -501,7 +502,7 @@ static int nvme_cmd_sync(BlockDriverState *bs, 
> >> NVMeQueuePair *q,
> >>      }
> >>      nvme_submit_command(q, req, cmd, nvme_cmd_sync_cb, &ret);
> >>  
> >> -    BDRV_POLL_WHILE(bs, ret == -EINPROGRESS);
> >> +    AIO_WAIT_WHILE(aio_context, ret == -EINPROGRESS);
> > 
> > Maybe I would have:
> > 
> >     AIO_WAIT_WHILE(bdrv_get_aio_context(bs), ret == -EINPROGRESS);
> 
> I extracted aio_context in this patch because in the following
> series it is passed by the caller as an argument to nvme_cmd_sync(),
> so this makes the next series simpler to review.

Make sense!

> 
> > 
> > But it doesn't matter, LGTM:
> > 
> > Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
> 
> Thanks!
> 
> > 
> >>      return ret;
> >>  }
> >>  
> >> -- 
> >> 2.26.2
> >>
> >>
> > 
>