[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v3 09/14] block: Move some bdrv_*_all() function
From: |
Max Reitz |
Subject: |
Re: [Qemu-block] [PATCH v3 09/14] block: Move some bdrv_*_all() functions to BB |
Date: |
Sat, 20 Feb 2016 14:46:41 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 |
On 17.02.2016 16:51, Kevin Wolf wrote:
> Am 16.02.2016 um 19:08 hat Max Reitz geschrieben:
>> Move bdrv_commit_all() and bdrv_flush_all() to the BlockBackend level.
>>
>> Signed-off-by: Max Reitz <address@hidden>
>> ---
>> block.c | 20 ------------
>> block/block-backend.c | 44
>> +++++++++++++++++++++++----
>> block/io.c | 20 ------------
>> include/block/block.h | 2 --
>> stubs/Makefile.objs | 2 +-
>> stubs/{bdrv-commit-all.c => blk-commit-all.c} | 4 +--
>> 6 files changed, 41 insertions(+), 51 deletions(-)
>> rename stubs/{bdrv-commit-all.c => blk-commit-all.c} (53%)
>>
[...]
>> diff --git a/block/block-backend.c b/block/block-backend.c
>> index a10fe44..a918c35 100644
>> --- a/block/block-backend.c
>> +++ b/block/block-backend.c
[...]
>> @@ -1361,5 +1356,42 @@ BlockBackendRootState
>> *blk_get_root_state(BlockBackend *blk)
>>
>> int blk_commit_all(void)
>> {
>> - return bdrv_commit_all();
>> + BlockBackend *blk = NULL;
>> +
>> + while ((blk = blk_all_next(blk)) != NULL) {
>> + AioContext *aio_context = blk_get_aio_context(blk);
>> +
>> + aio_context_acquire(aio_context);
>> + if (blk_is_available(blk) && blk->bs->drv && blk->bs->backing) {
>
> blk->bs->drv is already implied by blk_is_available(), so it's
> unnecessary, even though it doesn't actively hurt.
>
> Also, using blk_is_available() instead of blk_is_inserted() as in
> blk_flush_all() means that a drive with an open tray, but inserted
> medium isn't committed. I assume you did this on purpose because you're
> using two different functions in this patch, but isn't this a change in
> behaviour? If so, and we really want it, it should at least be mentioned
> in the commit message.
Drives with open trays are strange.
I found it reasonable that every medium in the system should be flushed
on blk_flush_all(), because flushing data should only bring the on-disk
state to a state it is supposed to be in anyway.
On the other hand, the commit operation actively changes data.
Therefore, I decided to treat it like a guest write operation. However,
considering that blk_commit_all() is basically only available through
HMP and is thus more of a legacy operation, I don't think its behavior
should be changed here. I'm therefore going to change this to
blk_is_inserted(), thanks.
Max
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-block] [PATCH v3 08/14] blockdev: Remove blk_hide_on_behalf_of_hmp_drive_del(), (continued)
[Qemu-block] [PATCH v3 07/14] blockdev: Add list of monitor-owned BlockBackends, Max Reitz, 2016/02/16
[Qemu-block] [PATCH v3 09/14] block: Move some bdrv_*_all() functions to BB, Max Reitz, 2016/02/16
[Qemu-block] [PATCH v3 10/14] block: Add bdrv_next_monitor_owned(), Max Reitz, 2016/02/16
[Qemu-block] [PATCH v3 12/14] block: Rewrite bdrv_next(), Max Reitz, 2016/02/16
[Qemu-block] [PATCH v3 11/14] block: Add blk_next_root_bs(), Max Reitz, 2016/02/16
[Qemu-block] [PATCH v3 13/14] block: Use bdrv_next() instead of bdrv_states, Max Reitz, 2016/02/16
[Qemu-block] [PATCH v3 14/14] block: Remove bdrv_states list, Max Reitz, 2016/02/16
Re: [Qemu-block] [PATCH v3 00/14] blockdev: Further BlockBackend work, Kevin Wolf, 2016/02/17