On 25/10/2016 15:39, Alberto Garcia wrote: > On Mon 24 Oct 2016 12:53:41 PM CEST, Paolo Bonzini wrote: > >>> My first thoughts were about how to let an unpause succeed without a >>> previous pause for these objects, but actually I think this isn't >>> what we should do. We rather want to actually do the pause instead >>> because even new BDSes and block jobs should probably start in a >>> quiesced state when created inside a drain_all section. >> >> Yes, I agree with this. It shouldn't be too hard to implement it. It >> would require a BlockDriverState to look at the global "inside >> bdrv_drain_all_begin" state, and ask its BlockBackend to disable >> itself upon bdrv_replace_child. > > Why in bdrv_replace_child()? bdrv_drain_all_end() enables all BDSs, but > if you add one with "blockdev-add" it's not going to be disabled using > this method.
You only need to disable it when blk_insert_bs is called. In fact... > In addition to that block jobs need the same, don't they? Something like > "job->pause_count = all_quiesce_counter" in the initialization. ... we discussed a couple weeks ago that block jobs should register BlkDeviceOps that pause the job in the drained_begin callback. So when the block job calls blk_insert_bs, the drained_begin callback would start the job as paused. > I think we'd also need to add block_job_pause_point() at the beginning > of each one of their coroutines, in order to make sure that they really > start paused. It depends on the job, for example streaming starts with block_job_sleep_ns. Commit also does, except for some blk_getlength and blk_truncate calls that could be moved to the caller. Paolo
