[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v11 01/19] block: Add bdrv_drain_all_{begin, end
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH v11 01/19] block: Add bdrv_drain_all_{begin, end}() |
Date: |
Tue, 25 Oct 2016 16:41:23 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 |
On 25/10/2016 16:38, Kevin Wolf wrote:
> Am 25.10.2016 um 15:53 hat Paolo Bonzini geschrieben:
>>
>>
>> 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...
>
> This assumes that the block driver doesn't issue internal background I/O
> by itself. Probably true for everything that we have today, but it would
> probably be cleaner to quiesce it directly in bdrv_open_common().
So
bs->quiesce_counter = all_quiesce_counter;
? That would work. Should bdrv_close assert bs->quiesce_counter==0
(which implies all_quiesce_counter == 0), since it usually has to do I/O?
>>> 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.
>
> Yes, should, but doing this kind of infrastructure work isn't something
> for this series.
I agree. I was just explaining why it wouldn't be necessary to
initialize job->pause_count.
Paolo
>>> 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.
>
> Kevin
>
- Re: [Qemu-devel] [PATCH v11 01/19] block: Add bdrv_drain_all_{begin, end}(), (continued)
- Re: [Qemu-devel] [PATCH v11 01/19] block: Add bdrv_drain_all_{begin, end}(), Kevin Wolf, 2016/10/19
- Re: [Qemu-devel] [PATCH v11 01/19] block: Add bdrv_drain_all_{begin, end}(), Alberto Garcia, 2016/10/20
- Re: [Qemu-devel] [PATCH v11 01/19] block: Add bdrv_drain_all_{begin, end}(), Kevin Wolf, 2016/10/20
- Re: [Qemu-devel] [PATCH v11 01/19] block: Add bdrv_drain_all_{begin, end}(), John Snow, 2016/10/21
- Re: [Qemu-devel] [PATCH v11 01/19] block: Add bdrv_drain_all_{begin, end}(), Alberto Garcia, 2016/10/21
- Re: [Qemu-devel] [PATCH v11 01/19] block: Add bdrv_drain_all_{begin, end}(), John Snow, 2016/10/21
- Re: [Qemu-devel] [PATCH v11 01/19] block: Add bdrv_drain_all_{begin, end}(), Paolo Bonzini, 2016/10/24
- Re: [Qemu-devel] [PATCH v11 01/19] block: Add bdrv_drain_all_{begin, end}(), Alberto Garcia, 2016/10/25
- Re: [Qemu-devel] [PATCH v11 01/19] block: Add bdrv_drain_all_{begin, end}(), Paolo Bonzini, 2016/10/25
- Re: [Qemu-devel] [PATCH v11 01/19] block: Add bdrv_drain_all_{begin, end}(), Kevin Wolf, 2016/10/25
- Re: [Qemu-devel] [PATCH v11 01/19] block: Add bdrv_drain_all_{begin, end}(),
Paolo Bonzini <=
- Re: [Qemu-devel] [PATCH v11 01/19] block: Add bdrv_drain_all_{begin, end}(), Kevin Wolf, 2016/10/25
- Re: [Qemu-devel] [PATCH v11 01/19] block: Add bdrv_drain_all_{begin, end}(), Alberto Garcia, 2016/10/25
- Re: [Qemu-devel] [PATCH v11 01/19] block: Add bdrv_drain_all_{begin, end}(), Paolo Bonzini, 2016/10/25
[Qemu-devel] [PATCH v11 06/19] block: Check blockers in all nodes involved in a block-commit job, Alberto Garcia, 2016/10/14
[Qemu-devel] [PATCH v11 03/19] block: Add block_job_add_bdrv(), Alberto Garcia, 2016/10/14
[Qemu-devel] [PATCH v11 12/19] qemu-iotests: Test streaming to an intermediate layer, Alberto Garcia, 2016/10/14
[Qemu-devel] [PATCH v11 02/19] block: Pause all jobs during bdrv_reopen_multiple(), Alberto Garcia, 2016/10/14
[Qemu-devel] [PATCH v11 07/19] block: Block all nodes involved in the block-commit operation, Alberto Garcia, 2016/10/14
[Qemu-devel] [PATCH v11 05/19] block: Use block_job_add_bdrv() in backup_start(), Alberto Garcia, 2016/10/14