diff mbox

[03/27] block/parallels: switch to bdrv_read

Message ID 1426069701-1405-4-git-send-email-den@openvz.org
State New
Headers show

Commit Message

Denis V. Lunev March 11, 2015, 10:27 a.m. UTC 
From: Roman Kagan <rkagan@parallels.com>

Switch the .bdrv_read method implementation from using bdrv_pread() to
bdrv_read() on the underlying file, since the latter is subject to i/o
throttling while the former is not.

Besides, since bdrv_read() operates in sectors rather than bytes, adjust
the helper functions to do so too.

Signed-off-by: Roman Kagan <rkagan@parallels.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/parallels.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

Comments

Stefan Hajnoczi April 22, 2015, 12:23 p.m. UTC | #1 
On Wed, Mar 11, 2015 at 01:27:57PM +0300, Denis V. Lunev wrote:
> From: Roman Kagan <rkagan@parallels.com>
> 
> Switch the .bdrv_read method implementation from using bdrv_pread() to
> bdrv_read() on the underlying file, since the latter is subject to i/o
> throttling while the former is not.
> 
> Besides, since bdrv_read() operates in sectors rather than bytes, adjust
> the helper functions to do so too.

Can this patch be dropped since "[PATCH 06/27] block/parallels: provide
_co_readv routine for parallels format driver" switches to
.bdrv_co_readv() anyway?

Stefan
Denis V. Lunev April 22, 2015, 12:30 p.m. UTC | #2 
On 22/04/15 15:23, Stefan Hajnoczi wrote:
> On Wed, Mar 11, 2015 at 01:27:57PM +0300, Denis V. Lunev wrote:
>> From: Roman Kagan <rkagan@parallels.com>
>>
>> Switch the .bdrv_read method implementation from using bdrv_pread() to
>> bdrv_read() on the underlying file, since the latter is subject to i/o
>> throttling while the former is not.
>>
>> Besides, since bdrv_read() operates in sectors rather than bytes, adjust
>> the helper functions to do so too.
> Can this patch be dropped since "[PATCH 06/27] block/parallels: provide
> _co_readv routine for parallels format driver" switches to
> .bdrv_co_readv() anyway?
>
> Stefan
this is not that easy. This patch changes seek_to_sector call and
in reality to drop it I have to merge patches 3, 4 and 6.
Besides that this patch has been written by my colleague and
I feel comfortable to preserve original Signed-off-by of him.

Den
Stefan Hajnoczi April 23, 2015, 8:48 a.m. UTC | #3 
On Wed, Apr 22, 2015 at 03:30:50PM +0300, Denis V. Lunev wrote:
> On 22/04/15 15:23, Stefan Hajnoczi wrote:
> >On Wed, Mar 11, 2015 at 01:27:57PM +0300, Denis V. Lunev wrote:
> >>From: Roman Kagan <rkagan@parallels.com>
> >>
> >>Switch the .bdrv_read method implementation from using bdrv_pread() to
> >>bdrv_read() on the underlying file, since the latter is subject to i/o
> >>throttling while the former is not.
> >>
> >>Besides, since bdrv_read() operates in sectors rather than bytes, adjust
> >>the helper functions to do so too.
> >Can this patch be dropped since "[PATCH 06/27] block/parallels: provide
> >_co_readv routine for parallels format driver" switches to
> >.bdrv_co_readv() anyway?
> >
> >Stefan
> this is not that easy. This patch changes seek_to_sector call and
> in reality to drop it I have to merge patches 3, 4 and 6.
> Besides that this patch has been written by my colleague and
> I feel comfortable to preserve original Signed-off-by of him.

I understand.

Stefan
diff mbox

Patch

diff --git a/block/parallels.c b/block/parallels.c
index dca0df6..baefd3e 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -146,9 +146,8 @@  fail:
     return ret;
 }
 
-static int64_t seek_to_sector(BlockDriverState *bs, int64_t sector_num)
+static int64_t seek_to_sector(BDRVParallelsState *s, int64_t sector_num)
 {
-    BDRVParallelsState *s = bs->opaque;
     uint32_t index, offset;
 
     index = sector_num / s->tracks;
@@ -157,24 +156,27 @@  static int64_t seek_to_sector(BlockDriverState *bs, int64_t sector_num)
     /* not allocated */
     if ((index >= s->catalog_size) || (s->catalog_bitmap[index] == 0))
         return -1;
-    return
-        ((uint64_t)s->catalog_bitmap[index] * s->off_multiplier + offset) * 512;
+    return (uint64_t)s->catalog_bitmap[index] * s->off_multiplier + offset;
 }
 
 static int parallels_read(BlockDriverState *bs, int64_t sector_num,
                     uint8_t *buf, int nb_sectors)
 {
+    BDRVParallelsState *s = bs->opaque;
+
     while (nb_sectors > 0) {
-        int64_t position = seek_to_sector(bs, sector_num);
+        int64_t position = seek_to_sector(s, sector_num);
         if (position >= 0) {
-            if (bdrv_pread(bs->file, position, buf, 512) != 512)
-                return -1;
+            int ret = bdrv_read(bs->file, position, buf, 1);
+            if (ret < 0) {
+                return ret;
+            }
         } else {
-            memset(buf, 0, 512);
+            memset(buf, 0, BDRV_SECTOR_SIZE);
         }
         nb_sectors--;
         sector_num++;
-        buf += 512;
+        buf += BDRV_SECTOR_SIZE;
     }
     return 0;
 }