Re: [PATCH v5 12/21] xen-block: implement BlockDevOps->drained_begin()

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Thu, May 04, 2023 at 03:53:18PM -0400, Stefan Hajnoczi wrote:
> @@ -819,11 +841,9 @@ void xen_block_dataplane_start(XenBlockDataPlane 
> *dataplane,
>      blk_set_aio_context(dataplane->blk, dataplane->ctx, NULL);
>      aio_context_release(old_context);
>  
> -    /* Only reason for failure is a NULL channel */
> -    aio_context_acquire(dataplane->ctx);
> -    xen_device_set_event_channel_context(xendev, dataplane->event_channel,
> -                                         dataplane->ctx, &error_abort);
> -    aio_context_release(dataplane->ctx);
> +    if (!blk_in_drain(dataplane->blk)) {

There's maybe something missing in the patch.
xen_block_dataplane_start() calls xen_device_bind_event_channel() just
before xen_block_dataplane_attach().

And xen_device_bind_event_channel() sets the event context to
qemu_get_aio_context() instead of NULL.

So, even if we don't call xen_block_dataplane_attach() while in drain,
there's already a fd_handler attach to the fd. So should
xen_device_bind_event_channel() be changed as well? Or maybe a call to
xen_block_dataplane_detach() would be enough.

(There's only one user of xen_device_bind_event_channel() at the moment
so I don't know if other implementation making use of this API will want
to call set_event_channel_context or not.)

> +        xen_block_dataplane_attach(dataplane);
> +    }
>  
>      return;
>  

-- 
Anthony PERARD