From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38575) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Uuegw-00039r-Uy for qemu-devel@nongnu.org; Thu, 04 Jul 2013 04:15:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UuedN-0004C7-JF for qemu-devel@nongnu.org; Thu, 04 Jul 2013 04:11:24 -0400 Received: from mail-wi0-x22f.google.com ([2a00:1450:400c:c05::22f]:44617) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UuedN-0004BI-2R for qemu-devel@nongnu.org; Thu, 04 Jul 2013 04:11:21 -0400 Received: by mail-wi0-f175.google.com with SMTP id m6so6021560wiv.8 for ; Thu, 04 Jul 2013 01:11:20 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <51D52E1F.2030405@redhat.com> Date: Thu, 04 Jul 2013 10:11:11 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1372862071-28225-1-git-send-email-pbonzini@redhat.com> <1372862071-28225-3-git-send-email-pbonzini@redhat.com> <20130704024036.GB6659@T430s.nay.redhat.com> In-Reply-To: <20130704024036.GB6659@T430s.nay.redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 02/17] cow: make writes go at a less indecent speed List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: famz@redhat.com Cc: kwolf@redhat.com, pl@kamp.de, qemu-devel@nongnu.org, stefanha@redhat.com 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 >> --- >> 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 >> >> >> >