[Qemu-devel] [PATCH resend] block: fix wrong order in live block migration setup

[Qemu-devel] [PATCH resend] block: fix wrong order in live block migration setup

[chai wen]

The function init_blk_migration is better to be called before
set_dirty_tracking as the reasons below.

If we want to track dirty blocks via dirty_maps on a BlockDriverState
when doing live block-migration, its correspoding 'BlkMigDevState' should be
added to block_mig_state.bmds_list first for subsequent processing.
Otherwise set_dirty_tracking will do nothing on an empty list than allocating
dirty_bitmaps for them. And bdrv_get_dirty_count will access the 
bmds->dirty_maps directly, then there would be a segfault triggered.

If the set_dirty_tracking fails, qemu_savevm_state_cancel will handle
the cleanup of init_blk_migration automatically.


Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: chai wen <chaiw.fnst@cn.fujitsu.com>
---
 block-migration.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/block-migration.c b/block-migration.c
index 1656270..25a0388 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -629,6 +629,7 @@ 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 */
     ret = set_dirty_tracking();
@@ -638,8 +639,6 @@ static int block_save_setup(QEMUFile *f, void *opaque)
         return ret;
     }
 
-    init_blk_migration(f);
-
     qemu_mutex_unlock_iothread();
 
     ret = flush_blks(f);
-- 
1.7.1

Re: [Qemu-devel] [PATCH resend] block: fix wrong order in live block migration setup

[Stefan Hajnoczi]

On Wed, Jun 04, 2014 at 11:47:37AM +0800, chai wen wrote:
> 
> The function init_blk_migration is better to be called before
> set_dirty_tracking as the reasons below.
> 
> If we want to track dirty blocks via dirty_maps on a BlockDriverState
> when doing live block-migration, its correspoding 'BlkMigDevState' should be
> added to block_mig_state.bmds_list first for subsequent processing.
> Otherwise set_dirty_tracking will do nothing on an empty list than allocating
> dirty_bitmaps for them. And bdrv_get_dirty_count will access the 
> bmds->dirty_maps directly, then there would be a segfault triggered.
> 
> If the set_dirty_tracking fails, qemu_savevm_state_cancel will handle
> the cleanup of init_blk_migration automatically.
> 
> 
> Reviewed-by: Fam Zheng <famz@redhat.com>
> Signed-off-by: chai wen <chaiw.fnst@cn.fujitsu.com>
> ---
>  block-migration.c |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

Re: [Qemu-devel] [PATCH resend] block: fix wrong order in live block migration setup

[Chai Wen]

On 06/04/2014 05:23 PM, Stefan Hajnoczi wrote:

> On Wed, Jun 04, 2014 at 11:47:37AM +0800, chai wen wrote:
>>
>> The function init_blk_migration is better to be called before
>> set_dirty_tracking as the reasons below.
>>
>> If we want to track dirty blocks via dirty_maps on a BlockDriverState
>> when doing live block-migration, its correspoding 'BlkMigDevState' should be
>> added to block_mig_state.bmds_list first for subsequent processing.
>> Otherwise set_dirty_tracking will do nothing on an empty list than allocating
>> dirty_bitmaps for them. And bdrv_get_dirty_count will access the 
>> bmds->dirty_maps directly, then there would be a segfault triggered.
>>
>> If the set_dirty_tracking fails, qemu_savevm_state_cancel will handle
>> the cleanup of init_blk_migration automatically.
>>
>>
>> Reviewed-by: Fam Zheng <famz@redhat.com>
>> Signed-off-by: chai wen <chaiw.fnst@cn.fujitsu.com>
>> ---
>>  block-migration.c |    3 +--
>>  1 files changed, 1 insertions(+), 2 deletions(-)
> 
> Thanks, applied to my block tree:
> https://github.com/stefanha/qemu/commits/block
> 


OK， thanks for your attention about this fix. :)

thanks
chai wen

> Stefan
> .
> 



-- 
Regards

Chai Wen