Re: [Qemu-devel] [PATCH v2 11/16] sheepdog: disable dataplane

[Kevin Wolf]

Am 16.03.2016 um 15:16 hat Paolo Bonzini geschrieben:
> sheepdog has some calls to aio_poll that are hard to eliminate, for
> example in sd_sheepdog_goto's call to do_req.  Since I don't have
> means to test sheepdog well, disable dataplane altogether for this
> driver.
> 
> Reviewed-by: Fam Zheng <address@hidden>
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  block/sheepdog.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/block/sheepdog.c b/block/sheepdog.c
> index a6e98a5..8ced3e5 100644
> --- a/block/sheepdog.c
> +++ b/block/sheepdog.c
> @@ -364,6 +364,7 @@ struct SheepdogAIOCB {
>  typedef struct BDRVSheepdogState {
>      BlockDriverState *bs;
>      AioContext *aio_context;
> +    Error *blocker;
>  
>      SheepdogInode inode;
>  
> @@ -1422,6 +1423,21 @@ static int sd_open(BlockDriverState *bs, QDict 
> *options, int flags,
>      Error *local_err = NULL;
>      const char *filename;
>  
> +    /* sd_snapshot_goto does blocking operations that call aio_poll
> +     * (through do_req).  This can cause races with iothread:
> +     *
> +     *       main thread                       I/O thread
> +     *       -----------------                 ------------------
> +     *       while(srco.finished == false)
> +     *                                         aio_poll(..., true)
> +     *                                            srco.finished = true
> +     *         aio_poll(..., true)
> +     *
> +     * Now aio_poll potentially blocks forever.
> +     */
> +    error_setg(&s->blocker, "sheepdog does not support iothreads");
> +    bdrv_op_block(bs, BLOCK_OP_TYPE_DATAPLANE, s->blocker);

Our current op blockers are weak, so this doesn't completely rule out
having a sheepdog BDS under a dataplane device.

Actually, did you check that even the most obvious case works? If we
have a format layer on top (which we do by default), I think attaching
to dataplane would still work because only the blockers of the top level
would be considered. We have to be sure about this one.

But there are other, less common cases that could still result in a bad
setup. Essentially, this just blocks enabling dataplane on a device that
has a sheepdog BDS (on top), but it doesn't block opening a sheepdog
backend for a device that has dataplane already enabled. This includes
scenarios with removable media like virtio-scsi CD-ROMs, but also live
snapshots or block jobs with a target image on sheepdog.

I wouldn't feel comfortable about ignoring this second part, but maybe
we could get away with it if we have a plan how to fix it in the long
run. The new op blockers should be able to do that, but I guess it will
be well into the 2.7 development cycle, if not later, before we have
them.

Kevin