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

[Kevin Wolf]

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

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

This doesn't seem to be the crucial difference between both.

> > If the answer is "because block jobs are already paused while draining"
> > (and probably nobody thought much about the NBD server), then chances
> > are that it's not the right thing.  In fact, using two different
> > mechanisms for pausing block jobs and pausing device I/O seems
> > inconsistent and wrong.
> > 
> > I suspect that the real solution needs to be in the block layer, though
> > I'm not quite sure yet what it would be like. Perhaps a function pair
> > like blk_stop/resume_io() that is used by bdrv_drain() callers and works
> > on the BlockBackend level.
> 
> 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?

Kevin