From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1O9eU1-0000p4-HD for qemu-devel@nongnu.org; Wed, 05 May 2010 09:17:49 -0400 Received: from [140.186.70.92] (port=39889 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1O9eU0-0000mx-4a for qemu-devel@nongnu.org; Wed, 05 May 2010 09:17:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1O9eTy-0001Qi-CM for qemu-devel@nongnu.org; Wed, 05 May 2010 09:17:48 -0400 Received: from mail-gy0-f173.google.com ([209.85.160.173]:48402) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1O9eTy-0001QZ-7H for qemu-devel@nongnu.org; Wed, 05 May 2010 09:17:46 -0400 Received: by gyd5 with SMTP id 5so2137604gyd.4 for ; Wed, 05 May 2010 06:17:45 -0700 (PDT) Message-ID: <4BE16FF3.1070700@codemonkey.ws> Date: Wed, 05 May 2010 08:17:39 -0500 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 1/6] bochs: use pread References: <20100504104408.GA6388@lst.de> In-Reply-To: <20100504104408.GA6388@lst.de> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Christoph Hellwig Cc: Kevin Wolf , qemu-devel@nongnu.org On 05/04/2010 05:44 AM, Christoph Hellwig wrote: > Use pread instead of lseek + read in preparation of using the qemu > block API. > > Signed-off-by: Christoph Hellwig > Looks good to me. Reviewed-by: Anthony Liguori Regards, Anthony Liguori > Index: qemu-kevin/block/bochs.c > =================================================================== > --- qemu-kevin.orig/block/bochs.c 2010-05-03 12:58:53.419012621 +0200 > +++ qemu-kevin/block/bochs.c 2010-05-03 12:59:13.873005360 +0200 > @@ -125,7 +125,7 @@ static int bochs_open(BlockDriverState * > > s->fd = fd; > > - if (read(fd,&bochs, sizeof(bochs)) != sizeof(bochs)) { > + if (pread(fd,&bochs, sizeof(bochs), 0) != sizeof(bochs)) { > goto fail; > } > > @@ -144,14 +144,10 @@ static int bochs_open(BlockDriverState * > bs->total_sectors = le64_to_cpu(bochs.extra.redolog.disk) / 512; > } > > - if (lseek(s->fd, le32_to_cpu(bochs.header), SEEK_SET) == (off_t)-1) { > - goto fail; > - } > - > s->catalog_size = le32_to_cpu(bochs.extra.redolog.catalog); > s->catalog_bitmap = qemu_malloc(s->catalog_size * 4); > - if (read(s->fd, s->catalog_bitmap, s->catalog_size * 4) != > - s->catalog_size * 4) > + if (pread(s->fd, s->catalog_bitmap, s->catalog_size * 4, > + le32_to_cpu(bochs.header)) != s->catalog_size * 4) > goto fail; > for (i = 0; i< s->catalog_size; i++) > le32_to_cpus(&s->catalog_bitmap[i]); > @@ -169,54 +165,35 @@ static int bochs_open(BlockDriverState * > return -1; > } > > -static inline int seek_to_sector(BlockDriverState *bs, int64_t sector_num) > +static int64_t seek_to_sector(BlockDriverState *bs, int64_t sector_num) > { > BDRVBochsState *s = bs->opaque; > int64_t offset = sector_num * 512; > - int64_t extent_index, extent_offset, bitmap_offset, block_offset; > + int64_t extent_index, extent_offset, bitmap_offset; > char bitmap_entry; > > // seek to sector > extent_index = offset / s->extent_size; > extent_offset = (offset % s->extent_size) / 512; > > - if (s->catalog_bitmap[extent_index] == 0xffffffff) > - { > -// fprintf(stderr, "page not allocated [%x - %x:%x]\n", > -// sector_num, extent_index, extent_offset); > - return -1; // not allocated > + if (s->catalog_bitmap[extent_index] == 0xffffffff) { > + return -1; /* not allocated */ > } > > bitmap_offset = s->data_offset + (512 * s->catalog_bitmap[extent_index] * > (s->extent_blocks + s->bitmap_blocks)); > - block_offset = bitmap_offset + (512 * (s->bitmap_blocks + extent_offset)); > > -// fprintf(stderr, "sect: %x [ext i: %x o: %x] -> %x bitmap: %x block: %x\n", > -// sector_num, extent_index, extent_offset, > -// le32_to_cpu(s->catalog_bitmap[extent_index]), > -// bitmap_offset, block_offset); > - > - // read in bitmap for current extent > - if (lseek(s->fd, bitmap_offset + (extent_offset / 8), SEEK_SET) == > - (off_t)-1) { > + /* read in bitmap for current extent */ > + if (pread(s->fd,&bitmap_entry, 1, bitmap_offset + (extent_offset / 8)) > + != 1) { > return -1; > } > > - if (read(s->fd,&bitmap_entry, 1) != 1) > - return -1; > - > - if (!((bitmap_entry>> (extent_offset % 8))& 1)) > - { > -// fprintf(stderr, "sector (%x) in bitmap not allocated\n", > -// sector_num); > - return -1; // not allocated > + if (!((bitmap_entry>> (extent_offset % 8))& 1)) { > + return -1; /* not allocated */ > } > > - if (lseek(s->fd, block_offset, SEEK_SET) == (off_t)-1) { > - return -1; > - } > - > - return 0; > + return bitmap_offset + (512 * (s->bitmap_blocks + extent_offset)); > } > > static int bochs_read(BlockDriverState *bs, int64_t sector_num, > @@ -226,13 +203,13 @@ static int bochs_read(BlockDriverState * > int ret; > > while (nb_sectors> 0) { > - if (!seek_to_sector(bs, sector_num)) > - { > - ret = read(s->fd, buf, 512); > - if (ret != 512) > - return -1; > - } > - else > + int64_t block_offset = seek_to_sector(bs, sector_num); > + if (block_offset>= 0) { > + ret = pread(s->fd, buf, 512, block_offset); > + if (ret != 512) { > + return -1; > + } > + } else > memset(buf, 0, 512); > nb_sectors--; > sector_num++; > > > >