diff mbox

[5/6] parallels: use pread

Message ID 20100504104504.GE6388@lst.de
State New
Headers show

Commit Message

Christoph Hellwig May 4, 2010, 10:45 a.m. UTC 
Use pread instead of lseek + read in preparation of using the qemu
block API.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Comments

Kevin Wolf May 5, 2010, 2:19 p.m. UTC | #1 
Am 04.05.2010 12:45, schrieb Christoph Hellwig:
> Use pread instead of lseek + read in preparation of using the qemu
> block API.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Index: qemu-kevin/block/parallels.c
> ===================================================================
> --- qemu-kevin.orig/block/parallels.c	2010-05-03 13:00:09.711253925 +0200
> +++ qemu-kevin/block/parallels.c	2010-05-03 13:04:15.686033993 +0200
> @@ -83,7 +83,7 @@ static int parallels_open(BlockDriverSta
>  
>      s->fd = fd;
>  
> -    if (read(fd, &ph, sizeof(ph)) != sizeof(ph))
> +    if (pread(fd, &ph, sizeof(ph), 0) != sizeof(ph))
>          goto fail;
>  
>      if (memcmp(ph.magic, HEADER_MAGIC, 16) ||
> @@ -93,14 +93,11 @@ static int parallels_open(BlockDriverSta
>  
>      bs->total_sectors = le32_to_cpu(ph.nb_sectors);
>  
> -    if (lseek(s->fd, 64, SEEK_SET) != 64)
> -	goto fail;
> -
>      s->tracks = le32_to_cpu(ph.tracks);
>  
>      s->catalog_size = le32_to_cpu(ph.catalog_entries);
>      s->catalog_bitmap = qemu_malloc(s->catalog_size * 4);
> -    if (read(s->fd, s->catalog_bitmap, s->catalog_size * 4) !=
> +    if (pread(s->fd, s->catalog_bitmap, s->catalog_size * 4, 64) !=
>  	s->catalog_size * 4)
>  	goto fail;
>      for (i = 0; i < s->catalog_size; i++)
> @@ -114,28 +111,18 @@ fail:
>      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)
>  {
>      BDRVParallelsState *s = bs->opaque;
>      uint32_t index, offset;
> -    uint64_t position;
>  
>      index = sector_num / s->tracks;
>      offset = sector_num % s->tracks;
>  
> -    // not allocated
> +    /* not allocated */
>      if ((index > s->catalog_size) || (s->catalog_bitmap[index] == 0))
>  	return -1;
> -
> -    position = (uint64_t)(s->catalog_bitmap[index] + offset) * 512;
> -
> -//    fprintf(stderr, "sector: %llx index=%x offset=%x pointer=%x position=%x\n",
> -//	sector_num, index, offset, s->catalog_bitmap[index], position);
> -
> -    if (lseek(s->fd, position, SEEK_SET) != position)
> -	return -1;
> -
> -    return 0;
> +    return (uint64_t)(s->catalog_bitmap[index] + offset) * 512;
>  }
>  
>  static int parallels_read(BlockDriverState *bs, int64_t sector_num,
> @@ -144,11 +131,13 @@ static int parallels_read(BlockDriverSta
>      BDRVParallelsState *s = bs->opaque;
>  
>      while (nb_sectors > 0) {
> -	if (!seek_to_sector(bs, sector_num)) {
> -	    if (read(s->fd, buf, 512) != 512)
> -		return -1;
> -	} else
> +        uint64_t position = seek_to_sector(bs, sector_num);
> +        if (position >= 0) {

position should be a signed int64_t, otherwise the condition is always true.

Kevin
Christoph Hellwig May 5, 2010, 6:11 p.m. UTC | #2 
On Wed, May 05, 2010 at 04:19:30PM +0200, Kevin Wolf wrote:
> position should be a signed int64_t, otherwise the condition is always true.

Indeed, I'll update the patch.
diff mbox

Patch

Index: qemu-kevin/block/parallels.c
===================================================================
--- qemu-kevin.orig/block/parallels.c	2010-05-03 13:00:09.711253925 +0200
+++ qemu-kevin/block/parallels.c	2010-05-03 13:04:15.686033993 +0200
@@ -83,7 +83,7 @@  static int parallels_open(BlockDriverSta
 
     s->fd = fd;
 
-    if (read(fd, &ph, sizeof(ph)) != sizeof(ph))
+    if (pread(fd, &ph, sizeof(ph), 0) != sizeof(ph))
         goto fail;
 
     if (memcmp(ph.magic, HEADER_MAGIC, 16) ||
@@ -93,14 +93,11 @@  static int parallels_open(BlockDriverSta
 
     bs->total_sectors = le32_to_cpu(ph.nb_sectors);
 
-    if (lseek(s->fd, 64, SEEK_SET) != 64)
-	goto fail;
-
     s->tracks = le32_to_cpu(ph.tracks);
 
     s->catalog_size = le32_to_cpu(ph.catalog_entries);
     s->catalog_bitmap = qemu_malloc(s->catalog_size * 4);
-    if (read(s->fd, s->catalog_bitmap, s->catalog_size * 4) !=
+    if (pread(s->fd, s->catalog_bitmap, s->catalog_size * 4, 64) !=
 	s->catalog_size * 4)
 	goto fail;
     for (i = 0; i < s->catalog_size; i++)
@@ -114,28 +111,18 @@  fail:
     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)
 {
     BDRVParallelsState *s = bs->opaque;
     uint32_t index, offset;
-    uint64_t position;
 
     index = sector_num / s->tracks;
     offset = sector_num % s->tracks;
 
-    // not allocated
+    /* not allocated */
     if ((index > s->catalog_size) || (s->catalog_bitmap[index] == 0))
 	return -1;
-
-    position = (uint64_t)(s->catalog_bitmap[index] + offset) * 512;
-
-//    fprintf(stderr, "sector: %llx index=%x offset=%x pointer=%x position=%x\n",
-//	sector_num, index, offset, s->catalog_bitmap[index], position);
-
-    if (lseek(s->fd, position, SEEK_SET) != position)
-	return -1;
-
-    return 0;
+    return (uint64_t)(s->catalog_bitmap[index] + offset) * 512;
 }
 
 static int parallels_read(BlockDriverState *bs, int64_t sector_num,
@@ -144,11 +131,13 @@  static int parallels_read(BlockDriverSta
     BDRVParallelsState *s = bs->opaque;
 
     while (nb_sectors > 0) {
-	if (!seek_to_sector(bs, sector_num)) {
-	    if (read(s->fd, buf, 512) != 512)
-		return -1;
-	} else
+        uint64_t position = seek_to_sector(bs, sector_num);
+        if (position >= 0) {
+            if (pread(s->fd, buf, 512, position) != 512)
+                return -1;
+        } else {
             memset(buf, 0, 512);
+        }
         nb_sectors--;
         sector_num++;
         buf += 512;