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