[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 10/11] blockdev: Block device IO during block

From: Paolo Bonzini
Subject: Re: [Qemu-devel] [PATCH v2 10/11] blockdev: Block device IO during blockdev-backup transaction
Date: Wed, 13 May 2015 14:21:12 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0

On 13/05/2015 19:28, Fam Zheng wrote:
> +    state->bs = bs;
> +    error_setg(&state->blocker, "blockdev-backup in progress");
> +    bdrv_op_block(bs, BLOCK_OP_TYPE_DEVICE_IO, state->blocker);
> +
>      qmp_blockdev_backup(backup->device, backup->target,
>                          backup->sync,
>                          backup->has_speed, backup->speed,
> @@ -1696,7 +1701,6 @@ static void blockdev_backup_prepare(BlkTransactionState 
> *common, Error **errp)
>          return;
>      }
> -    state->bs = bs;

I don't understand this.  Jobs could pause/resume themselves by adding a
DEVICE_IO notifier on the targets.

However, block backups is the one job that cannot do this, because I/O
on the source triggers I/O on the target.

So if we consider this idea worthwhile, and decide that pausing device
I/O on the target should pause the block job, the backup job actually
has to prevent *adding a DEVICE_IO blocker* on the target.  This
"meta-block" is not possible in your design, which is a pity because on
the surface it looked nicer than mine.

FWIW, my original idea was:

- bdrv_pause checks if there is an operation blocker for PAUSE.  if it
is there, it fails

- otherwise, bdrv_pause invokes a notifier list if this is the outermost
call. if not the outermost call, it does nothing

- bdrv_resume does the same, but does not need a blocker

- drive-backup should block PAUSE on its target

Also, should the blockers (either DEVICE_IO in your design, or PAUSE in
mine) be included in bdrv_op_block_all.  I would say no in your case;
here is the proof:

- block-backup doesn't like DEVICE_IO blockers on the target

- block-backup calls bdrv_op_block_all on the target

- hence, bdrv_op_block_all shouldn't block DEVICE_IO

block_job_create is another suspicious caller of bdrv_op_block_all.  It
probably shouldn't block neither PAUSE nor DEVICE_IO.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]