From patchwork Fri May 7 15:17:19 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christoph Hellwig X-Patchwork-Id: 51952 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [199.232.76.165]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id E2049B7D19 for ; Sat, 8 May 2010 06:16:17 +1000 (EST) Received: from localhost ([127.0.0.1]:54708 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OATy1-0000OX-Qt for incoming@patchwork.ozlabs.org; Fri, 07 May 2010 16:16:13 -0400 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1OAPJ2-0000VA-FM for qemu-devel@nongnu.org; Fri, 07 May 2010 11:17:36 -0400 Received: from [140.186.70.92] (port=55361 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OAPJ0-0000P1-8R for qemu-devel@nongnu.org; Fri, 07 May 2010 11:17:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OAPIn-0007MZ-GL for qemu-devel@nongnu.org; Fri, 07 May 2010 11:17:25 -0400 Received: from verein.lst.de ([213.95.11.210]:57568) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OAPIm-0007Lw-4J for qemu-devel@nongnu.org; Fri, 07 May 2010 11:17:21 -0400 Received: from verein.lst.de (localhost [127.0.0.1]) by verein.lst.de (8.12.3/8.12.3/Debian-7.1) with ESMTP id o47FHJWY015281 (version=TLSv1/SSLv3 cipher=EDH-RSA-DES-CBC3-SHA bits=168 verify=NO) for ; Fri, 7 May 2010 17:17:19 +0200 Received: (from hch@localhost) by verein.lst.de (8.12.3/8.12.3/Debian-7.2) id o47FHJGg015280 for qemu-devel@nongnu.org; Fri, 7 May 2010 17:17:19 +0200 Date: Fri, 7 May 2010 17:17:19 +0200 From: Christoph Hellwig To: qemu-devel@nongnu.org Message-ID: <20100507151719.GB15249@lst.de> References: <20100507151658.GA15132@lst.de> Mime-Version: 1.0 Content-Disposition: inline In-Reply-To: <20100507151658.GA15132@lst.de> User-Agent: Mutt/1.3.28i X-Scanned-By: MIMEDefang 2.39 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 2) Subject: [Qemu-devel] [PATCH 2/3] cow: stop using mmap X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org We don't have an equivalent to mmap in the qemu block API, so read and write the bitmap directly. At least in the dumb implementation added in this patch this is a lot less efficient, but it means cow can also work on windows, and over nbd or curl. And it fixes qemu-iotests testcase 012 which did not work properly due to issues with read-only mmap access. In addition we can also get rid of the now unused get_mmap_addr function. Signed-off-by: Christoph Hellwig Index: qemu/block/cow.c =================================================================== --- qemu.orig/block/cow.c 2010-05-07 16:58:13.614003848 +0200 +++ qemu/block/cow.c 2010-05-07 17:07:35.326034649 +0200 @@ -21,11 +21,9 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN * THE SOFTWARE. */ -#ifndef _WIN32 #include "qemu-common.h" #include "block_int.h" #include "module.h" -#include /**************************************************************/ /* COW block driver using file system holes */ @@ -45,9 +43,6 @@ struct cow_header_v2 { typedef struct BDRVCowState { int fd; - uint8_t *cow_bitmap; /* if non NULL, COW mappings are used first */ - uint8_t *cow_bitmap_addr; /* mmap address of cow_bitmap */ - int cow_bitmap_size; int64_t cow_sectors_offset; } BDRVCowState; @@ -68,6 +63,7 @@ static int cow_open(BlockDriverState *bs BDRVCowState *s = bs->opaque; int fd; struct cow_header_v2 cow_header; + int bitmap_size; int64_t size; fd = open(filename, O_RDWR | O_BINARY | O_LARGEFILE); @@ -94,61 +90,92 @@ static int cow_open(BlockDriverState *bs pstrcpy(bs->backing_file, sizeof(bs->backing_file), cow_header.backing_file); - /* mmap the bitmap */ - s->cow_bitmap_size = ((bs->total_sectors + 7) >> 3) + sizeof(cow_header); - s->cow_bitmap_addr = (void *)mmap(get_mmap_addr(s->cow_bitmap_size), - s->cow_bitmap_size, - PROT_READ | PROT_WRITE, - MAP_SHARED, s->fd, 0); - if (s->cow_bitmap_addr == MAP_FAILED) - goto fail; - s->cow_bitmap = s->cow_bitmap_addr + sizeof(cow_header); - s->cow_sectors_offset = (s->cow_bitmap_size + 511) & ~511; + bitmap_size = ((bs->total_sectors + 7) >> 3) + sizeof(cow_header); + s->cow_sectors_offset = (bitmap_size + 511) & ~511; return 0; fail: close(fd); return -1; } -static inline void cow_set_bit(uint8_t *bitmap, int64_t bitnum) +/* + * XXX(hch): right now these functions are extremly ineffcient. + * 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) { - bitmap[bitnum / 8] |= (1 << (bitnum%8)); + BDRVCowState *s = bs->opaque; + uint64_t offset = sizeof(struct cow_header_v2) + bitnum / 8; + uint8_t bitmap; + + if (pread(s->fd, &bitmap, sizeof(bitmap), offset) != + sizeof(bitmap)) { + return -errno; + } + + bitmap |= (1 << (bitnum % 8)); + + if (pwrite(s->fd, &bitmap, sizeof(bitmap), offset) != + sizeof(bitmap)) { + return -errno; + } + return 0; } -static inline int is_bit_set(const uint8_t *bitmap, int64_t bitnum) +static inline int is_bit_set(BlockDriverState *bs, int64_t bitnum) { - return !!(bitmap[bitnum / 8] & (1 << (bitnum%8))); -} + BDRVCowState *s = bs->opaque; + uint64_t offset = sizeof(struct cow_header_v2) + bitnum / 8; + uint8_t bitmap; + + if (pread(s->fd, &bitmap, sizeof(bitmap), offset) != + sizeof(bitmap)) { + return -errno; + } + return !!(bitmap & (1 << (bitnum % 8))); +} /* Return true if first block has been changed (ie. current version is * in COW file). Set the number of continuous blocks for which that * is true. */ -static inline int is_changed(uint8_t *bitmap, - int64_t sector_num, int nb_sectors, - int *num_same) +static int cow_is_allocated(BlockDriverState *bs, int64_t sector_num, + int nb_sectors, int *num_same) { int changed; - if (!bitmap || nb_sectors == 0) { + if (nb_sectors == 0) { *num_same = nb_sectors; return 0; } - changed = is_bit_set(bitmap, sector_num); + changed = is_bit_set(bs, sector_num); + if (changed < 0) { + return 0; /* XXX: how to return I/O errors? */ + } + for (*num_same = 1; *num_same < nb_sectors; (*num_same)++) { - if (is_bit_set(bitmap, sector_num + *num_same) != changed) + if (is_bit_set(bs, sector_num + *num_same) != changed) break; } return changed; } -static int cow_is_allocated(BlockDriverState *bs, int64_t sector_num, - int nb_sectors, int *pnum) +static void cow_update_bitmap(BlockDriverState *bs, int64_t sector_num, + int nb_sectors) { - BDRVCowState *s = bs->opaque; - return is_changed(s->cow_bitmap, sector_num, nb_sectors, pnum); + int error = 0; + int i; + + for (i = 0; i < nb_sectors; i++) { + error = cow_set_bit(bs, sector_num + i); + if (error) { + break; + } + } + + return errror; } static int cow_read(BlockDriverState *bs, int64_t sector_num, @@ -158,7 +185,7 @@ static int cow_read(BlockDriverState *bs int ret, n; while (nb_sectors > 0) { - if (is_changed(s->cow_bitmap, sector_num, nb_sectors, &n)) { + if (cow_is_allocated(bs, sector_num, nb_sectors, &n)) { ret = pread(s->fd, buf, n * 512, s->cow_sectors_offset + sector_num * 512); if (ret != n * 512) @@ -184,21 +211,19 @@ static int cow_write(BlockDriverState *b const uint8_t *buf, int nb_sectors) { BDRVCowState *s = bs->opaque; - int ret, i; + int ret; ret = pwrite(s->fd, buf, nb_sectors * 512, s->cow_sectors_offset + sector_num * 512); if (ret != nb_sectors * 512) return -1; - for (i = 0; i < nb_sectors; i++) - cow_set_bit(s->cow_bitmap, sector_num + i); - return 0; + + return cow_update_bitmap(bs, sector_num, nb_sectors); } static void cow_close(BlockDriverState *bs) { BDRVCowState *s = bs->opaque; - munmap((void *)s->cow_bitmap_addr, s->cow_bitmap_size); close(s->fd); } @@ -308,4 +333,3 @@ static void bdrv_cow_init(void) } block_init(bdrv_cow_init); -#endif Index: qemu/qemu-common.h =================================================================== --- qemu.orig/qemu-common.h 2010-05-06 21:49:22.167011390 +0200 +++ qemu/qemu-common.h 2010-05-07 16:58:54.894013975 +0200 @@ -163,9 +163,6 @@ void qemu_free(void *ptr); char *qemu_strdup(const char *str); char *qemu_strndup(const char *str, size_t size); -void *get_mmap_addr(unsigned long size); - - void qemu_mutex_lock_iothread(void); void qemu_mutex_unlock_iothread(void); Index: qemu/qemu-malloc.c =================================================================== --- qemu.orig/qemu-malloc.c 2010-05-06 21:49:22.174010343 +0200 +++ qemu/qemu-malloc.c 2010-05-07 16:58:54.900253673 +0200 @@ -32,11 +32,6 @@ static void *oom_check(void *ptr) return ptr; } -void *get_mmap_addr(unsigned long size) -{ - return NULL; -} - void qemu_free(void *ptr) { free(ptr);