[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 02/17] cow: make writes go at a less indecent sp
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH 02/17] cow: make writes go at a less indecent speed |
Date: |
Thu, 04 Jul 2013 10:11:11 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130514 Thunderbird/17.0.6 |
Il 04/07/2013 04:40, Fam Zheng ha scritto:
> On Wed, 07/03 16:34, Paolo Bonzini wrote:
>> Only sync once per write, rather than once per sector.
>>
>> Signed-off-by: Paolo Bonzini <address@hidden>
>> ---
>> block/cow.c | 19 ++++++++++++++++---
>> 1 file changed, 16 insertions(+), 3 deletions(-)
>>
>> diff --git a/block/cow.c b/block/cow.c
>> index 204451e..133e596 100644
>> --- a/block/cow.c
>> +++ b/block/cow.c
>> @@ -106,7 +106,7 @@ static int cow_open(BlockDriverState *bs, QDict
>> *options, int flags)
>> * XXX(hch): right now these functions are extremely inefficient.
>> * We should just read the whole bitmap we'll need in one go instead.
>> */
>> -static inline int cow_set_bit(BlockDriverState *bs, int64_t bitnum)
>> +static inline int cow_set_bit(BlockDriverState *bs, int64_t bitnum, bool
>> *first)
> Why flush _before first_ write, rather than (more intuitively) flush
> _after last_ write?
Because you have to flush the data before you start writing the
metadata. Flushing the metadata can be done when the guest issues a flush.
This ensures that, in case of a power loss, the metadata will never
refer to data that hasn't been written.
Paolo
And personally I think "bool sync" makes a better
> signature than "bool *first", although it's not that critical as
> cow_update_bitmap is the only caller.
>> {
>> uint64_t offset = sizeof(struct cow_header_v2) + bitnum / 8;
>> uint8_t bitmap;
>> @@ -117,9 +117,21 @@ static inline int cow_set_bit(BlockDriverState *bs,
>> int64_t bitnum)
>> return ret;
>> }
>>
>> + if (bitmap & (1 << (bitnum % 8))) {
>> + return 0;
>> + }
>> +
>> + if (*first) {
>> + ret = bdrv_flush(bs->file);
>> + if (ret < 0) {
>> + return ret;
>> + }
>> + *first = false;
>> + }
>> +
>> bitmap |= (1 << (bitnum % 8));
>>
>> - ret = bdrv_pwrite_sync(bs->file, offset, &bitmap, sizeof(bitmap));
>> + ret = bdrv_pwrite(bs->file, offset, &bitmap, sizeof(bitmap));
>> if (ret < 0) {
>> return ret;
>> }
>> @@ -181,9 +193,10 @@ static int cow_update_bitmap(BlockDriverState *bs,
>> int64_t sector_num,
>> {
>> int error = 0;
>> int i;
>> + bool first = true;
>>
>> for (i = 0; i < nb_sectors; i++) {
>> - error = cow_set_bit(bs, sector_num + i);
>> + error = cow_set_bit(bs, sector_num + i, &first);
>> if (error) {
>> break;
>> }
>> --
>> 1.8.2.1
>>
>>
>>
>
- [Qemu-devel] [PATCH 00/17] Add qemu-img subcommand to dump file metadata, Paolo Bonzini, 2013/07/03
- [Qemu-devel] [PATCH 01/17] cow: make reads go at a decent speed, Paolo Bonzini, 2013/07/03
- [Qemu-devel] [PATCH 02/17] cow: make writes go at a less indecent speed, Paolo Bonzini, 2013/07/03
- [Qemu-devel] [PATCH 03/17] cow: do not call bdrv_co_is_allocated, Paolo Bonzini, 2013/07/03
- [Qemu-devel] [PATCH 04/17] block: make bdrv_co_is_allocated static, Paolo Bonzini, 2013/07/03
- [Qemu-devel] [PATCH 05/17] block: remove bdrv_is_allocated_above/bdrv_co_is_allocated_above distinction, Paolo Bonzini, 2013/07/03
- [Qemu-devel] [PATCH 07/17] qemu-img: always probe the input image for allocated sectors, Paolo Bonzini, 2013/07/03
- [Qemu-devel] [PATCH 06/17] block: expect errors from bdrv_co_is_allocated, Paolo Bonzini, 2013/07/03
- [Qemu-devel] [PATCH 08/17] block: make bdrv_has_zero_init return false for copy-on-write-images, Paolo Bonzini, 2013/07/03
- [Qemu-devel] [PATCH 09/17] block: introduce bdrv_get_block_status API, Paolo Bonzini, 2013/07/03
- [Qemu-devel] [PATCH 10/17] block: define get_block_status return value, Paolo Bonzini, 2013/07/03