diff mbox

[v3] block: Handle error of bdrv_getlength in bdrv_create_dirty_bitmap

Message ID 1397612070-11487-1-git-send-email-famz@redhat.com
State New
Headers show

Commit Message

Fam Zheng April 16, 2014, 1:34 a.m. UTC
bdrv_getlength could fail, check the return value before using it.
Return NULL and set errno if it fails. Callers are updated to handle
the error case.

Signed-off-by: Fam Zheng <famz@redhat.com>

---
v3: Fix errno (Max)

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block-migration.c     | 30 ++++++++++++++++++++++++++----
 block.c               | 11 +++++++++--
 block/mirror.c        |  5 ++++-
 include/block/block.h |  3 ++-
 4 files changed, 41 insertions(+), 8 deletions(-)

Comments

Max Reitz April 16, 2014, 9:36 p.m. UTC | #1
On 16.04.2014 03:34, Fam Zheng wrote:
> bdrv_getlength could fail, check the return value before using it.
> Return NULL and set errno if it fails. Callers are updated to handle
> the error case.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
>
> ---
> v3: Fix errno (Max)
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>   block-migration.c     | 30 ++++++++++++++++++++++++++----
>   block.c               | 11 +++++++++--
>   block/mirror.c        |  5 ++++-
>   include/block/block.h |  3 ++-
>   4 files changed, 41 insertions(+), 8 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>
Kevin Wolf April 17, 2014, 8:49 a.m. UTC | #2
Am 16.04.2014 um 23:36 hat Max Reitz geschrieben:
> On 16.04.2014 03:34, Fam Zheng wrote:
> >bdrv_getlength could fail, check the return value before using it.
> >Return NULL and set errno if it fails. Callers are updated to handle
> >the error case.
> >
> >Signed-off-by: Fam Zheng <famz@redhat.com>
> >
> >---
> >v3: Fix errno (Max)
> >
> >Signed-off-by: Fam Zheng <famz@redhat.com>
> >---
> >  block-migration.c     | 30 ++++++++++++++++++++++++++----
> >  block.c               | 11 +++++++++--
> >  block/mirror.c        |  5 ++++-
> >  include/block/block.h |  3 ++-
> >  4 files changed, 41 insertions(+), 8 deletions(-)
> 
> Reviewed-by: Max Reitz <mreitz@redhat.com>

Thanks, applied to the block branch.

Kevin
diff mbox

Patch

diff --git a/block-migration.c b/block-migration.c
index 897fdba..56951e0 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -310,13 +310,28 @@  static int mig_save_device_bulk(QEMUFile *f, BlkMigDevState *bmds)
 
 /* Called with iothread lock taken.  */
 
-static void set_dirty_tracking(void)
+static int set_dirty_tracking(void)
 {
     BlkMigDevState *bmds;
+    int ret;
 
     QSIMPLEQ_FOREACH(bmds, &block_mig_state.bmds_list, entry) {
-        bmds->dirty_bitmap = bdrv_create_dirty_bitmap(bmds->bs, BLOCK_SIZE);
+        bmds->dirty_bitmap = bdrv_create_dirty_bitmap(bmds->bs, BLOCK_SIZE,
+                                                      NULL);
+        if (!bmds->dirty_bitmap) {
+            ret = -errno;
+            goto fail;
+        }
     }
+    return 0;
+
+fail:
+    QSIMPLEQ_FOREACH(bmds, &block_mig_state.bmds_list, entry) {
+        if (bmds->dirty_bitmap) {
+            bdrv_release_dirty_bitmap(bmds->bs, bmds->dirty_bitmap);
+        }
+    }
+    return ret;
 }
 
 static void unset_dirty_tracking(void)
@@ -611,10 +626,17 @@  static int block_save_setup(QEMUFile *f, void *opaque)
             block_mig_state.submitted, block_mig_state.transferred);
 
     qemu_mutex_lock_iothread();
-    init_blk_migration(f);
 
     /* start track dirty blocks */
-    set_dirty_tracking();
+    ret = set_dirty_tracking();
+
+    if (ret) {
+        qemu_mutex_unlock_iothread();
+        return ret;
+    }
+
+    init_blk_migration(f);
+
     qemu_mutex_unlock_iothread();
 
     ret = flush_blks(f);
diff --git a/block.c b/block.c
index 990a754..11c4f82 100644
--- a/block.c
+++ b/block.c
@@ -5096,7 +5096,8 @@  bool bdrv_qiov_is_aligned(BlockDriverState *bs, QEMUIOVector *qiov)
     return true;
 }
 
-BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, int granularity)
+BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, int granularity,
+                                          Error **errp)
 {
     int64_t bitmap_size;
     BdrvDirtyBitmap *bitmap;
@@ -5105,7 +5106,13 @@  BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, int granularity)
 
     granularity >>= BDRV_SECTOR_BITS;
     assert(granularity);
-    bitmap_size = (bdrv_getlength(bs) >> BDRV_SECTOR_BITS);
+    bitmap_size = bdrv_getlength(bs);
+    if (bitmap_size < 0) {
+        error_setg_errno(errp, -bitmap_size, "could not get length of device");
+        errno = -bitmap_size;
+        return NULL;
+    }
+    bitmap_size >>= BDRV_SECTOR_BITS;
     bitmap = g_malloc0(sizeof(BdrvDirtyBitmap));
     bitmap->bitmap = hbitmap_alloc(bitmap_size, ffs(granularity) - 1);
     QLIST_INSERT_HEAD(&bs->dirty_bitmaps, bitmap, list);
diff --git a/block/mirror.c b/block/mirror.c
index 0ef41f9..2618c37 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -605,7 +605,10 @@  static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
     s->granularity = granularity;
     s->buf_size = MAX(buf_size, granularity);
 
-    s->dirty_bitmap = bdrv_create_dirty_bitmap(bs, granularity);
+    s->dirty_bitmap = bdrv_create_dirty_bitmap(bs, granularity, errp);
+    if (!s->dirty_bitmap) {
+        return;
+    }
     bdrv_set_enable_write_cache(s->target, true);
     bdrv_set_on_error(s->target, on_target_error, on_target_error);
     bdrv_iostatus_enable(s->target);
diff --git a/include/block/block.h b/include/block/block.h
index b3230a2..2b51eec 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -429,7 +429,8 @@  bool bdrv_qiov_is_aligned(BlockDriverState *bs, QEMUIOVector *qiov);
 
 struct HBitmapIter;
 typedef struct BdrvDirtyBitmap BdrvDirtyBitmap;
-BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, int granularity);
+BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, int granularity,
+                                          Error **errp);
 void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
 BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
 int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t sector);