Re: [Qemu-devel] [PATCH v6 01/13] block: Add op blocker type "device IO"

[Paolo Bonzini <pbonzini@redhat.com>]

On 27/05/2015 12:10, Kevin Wolf wrote:
> Am 27.05.2015 um 11:50 hat Paolo Bonzini geschrieben:
>>
>>
>> On 27/05/2015 11:07, Kevin Wolf wrote:
>>> This is the first part of what's troubling me with this series, as it
>>> makes me doubtful if op blockers are the right tool to implement what
>>> the commit message says (block device I/O). This is "are we doing the
>>> thing right?"
>>>
>>> The second part should actually come first, though: "Are we doing the
>>> right thing?" I'm also doubtful whether blocking device I/O is even what
>>> we should do.
>>>
>>> Why is device I/O special compared to block job I/O or NBD server I/O?
>>
>> Because block job I/O doesn't modify the source disk.  For the target
>> disk, block jobs should definitely treat themselves as device I/O and
>> register notifiers that pause themselves on bdrv_drain.
> 
> Block jobs don't generally have a source and a target; in fact, the
> design didn't even consider that such jobs exist (otherwise, we wouldn't
> have bs->job).

Mirror and backup have targets.

> There are some jobs that use a BDS read-only (source for backup, mirror,
> commit) just like there are guest devices that use the BDS read-only
> (CD-ROMs). And others write to it (streaming, hard disks).

Streaming doesn't write to the BDS.  It reads it while copy-on-read is
active.  It's subtle, but it means that streaming can proceed without
changing the contents of the disk.  As far as safety of atomic
transactions is concerned, streaming need not be paused.

However, you're right that there is no particular reason why a more
generic pause/resume wouldn't pause jobs as well.  It wouldn't be a
problem either way as far as the job is concerned, and a better-defined
API is a better API.

>> This is suspiciously similar to the first idea that I and Stefan had,
>> which was a blk_pause/blk_resume API, implemented through a notifier list.
> 
> Any problems with it because of which Fam decided against it?

I don't know, but I don't think so.

Paolo