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

[Paolo Bonzini]

On 13/05/2015 16:55, Fam Zheng wrote:
> > 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.
> 
> When do we need to pause device IO on the target? For drive-backup the target
> is anonymous and no device exists on it, so it's out of question. For
> blockdev-backup, users can have a device attached, but I'm not sure we should
> pause the job in this case.

Well, it depends on the meaning you give to "pause"/"device_io blocker".
 For me it was "I want exclusive ownership of the disk, no one else
should write".

For blockdev-backup for example you could mirror the backup destination
somewhere else, and mirror needs to drain the source.  For that to make
sense, the block job has to be paused.

Of course this is contrived, but it's better to make the design generic.

> > > 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
> 
> It's more complicated, because bdrv_drain_all can't fail now, if we want
> bdrv_pause there, then it could fail, and will become much harder to use.

I think we don't need bdrv_pause there.  See my reply to patch 11---we
want it in the callers.

> In your idea, who will block PAUSE, and why?

For example, blockdev-backup/drive-backup can block PAUSE on the target.
 But maybe they only need to add an op blocker notifier, and use it to
block device I/O on the source and drain it.  So your idea is safe.  Good!

Another idea was to block PAUSE in virtio-scsi until it's fixed.

Paolo

> > > - 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