Message ID | 1348217255-22441-39-git-send-email-quintela@redhat.com |
---|---|
State | New |
Headers | show |
Il 21/09/2012 10:47, Juan Quintela ha scritto: > Signed-off-by: Juan Quintela <quintela@redhat.com> > --- > block-migration.c | 21 +++++++++++---------- > 1 file changed, 11 insertions(+), 10 deletions(-) > > diff --git a/block-migration.c b/block-migration.c > index 565628f..9f94733 100644 > --- a/block-migration.c > +++ b/block-migration.c > @@ -423,10 +423,9 @@ static int mig_save_device_dirty(QEMUFile *f, BlkMigDevState *bmds, > > error: > DPRINTF("Error reading sector %" PRId64 "\n", sector); > - qemu_file_set_error(f, ret); > g_free(blk->buf); > g_free(blk); > - return 0; > + return ret; > } > > /* return value: > @@ -440,7 +439,7 @@ static int blk_mig_save_dirty_block(QEMUFile *f, int is_async) > > QSIMPLEQ_FOREACH(bmds, &block_mig_state.bmds_list, entry) { > ret = mig_save_device_dirty(f, bmds, is_async); > - if (ret == 0) { > + if (ret <= 0) { > break; > } > } > @@ -598,12 +597,17 @@ static int block_save_iterate(QEMUFile *f, void *opaque) > block_mig_state.bulk_completed = 1; > } > } else { > - if (blk_mig_save_dirty_block(f, 1) != 0) { > + ret = blk_mig_save_dirty_block(f, 1); > + if (ret != 0) { > /* no more dirty blocks */ > break; > } > } > } > + if (ret) { > + blk_mig_cleanup(); > + return ret; > + } > > ret = flush_blks(f); > if (ret) { > @@ -635,18 +639,15 @@ static int block_save_complete(QEMUFile *f, void *opaque) > all async read completed */ > assert(block_mig_state.submitted == 0); > Not clear what the fixes are, I'll take it as a cleanup. Then: > - while (blk_mig_save_dirty_block(f, 0) == 0) { > + while ((ret = blk_mig_save_dirty_block(f, 0)) == 0) { > /* Do nothing */ > } use a do...while instead: do { ret = blk_mig_save_dirty_block(f, 0); } while (ret == 0); > blk_mig_cleanup(); > - > - /* report completion */ > - qemu_put_be64(f, (100 << BDRV_SECTOR_BITS) | BLK_MIG_FLAG_PROGRESS); > - > - ret = qemu_file_get_error(f); > if (ret) { > return ret; > } Is it correct that we now return 1? We used to return 0. Paolo > + /* report completion */ > + qemu_put_be64(f, (100 << BDRV_SECTOR_BITS) | BLK_MIG_FLAG_PROGRESS); > > DPRINTF("Block migration completed\n"); >
Paolo Bonzini <pbonzini@redhat.com> wrote: > Il 21/09/2012 10:47, Juan Quintela ha scritto: >> Signed-off-by: Juan Quintela <quintela@redhat.com> >> --- >> @@ -635,18 +639,15 @@ static int block_save_complete(QEMUFile *f, void *opaque) >> all async read completed */ >> assert(block_mig_state.submitted == 0); >> > > Not clear what the fixes are, I'll take it as a cleanup. Then: We are returning now the errors directly. Until now, we were > >> - while (blk_mig_save_dirty_block(f, 0) == 0) { >> + while ((ret = blk_mig_save_dirty_block(f, 0)) == 0) { >> /* Do nothing */ >> } > > use a do...while instead: > > do { > ret = blk_mig_save_dirty_block(f, 0); > } while (ret == 0); I wanted to minimize the changes, but I don't really care, > >> blk_mig_cleanup(); >> - >> - /* report completion */ >> - qemu_put_be64(f, (100 << BDRV_SECTOR_BITS) | BLK_MIG_FLAG_PROGRESS); >> - >> - ret = qemu_file_get_error(f); >> if (ret) { >> return ret; >> } > > Is it correct that we now return 1? We used to return 0. If you look at the whole change, old code was: while (blk_mig_save_dirty_block(f, 0) != 0) { /* Do nothing */ } blk_mig_cleanup(); /* report completion */ qemu_put_be64(f, (100 << BDRV_SECTOR_BITS) | BLK_MIG_FLAG_PROGRESS); ret = qemu_file_get_error(f); if (ret) { return ret; } DPRINTF("Block migration completed\n"); qemu_put_be64(f, BLK_MIG_FLAG_EOS); return 0; i.e. if there is one error, we return it, otherwise, we return 0. while ((ret = blk_mig_save_dirty_block(f, 0)) == 0) { /* Do nothing */ } blk_mig_cleanup(); if (ret) { return ret; } /* report completion */ qemu_put_be64(f, (100 << BDRV_SECTOR_BITS) | BLK_MIG_FLAG_PROGRESS); DPRINTF("Block migration completed\n"); qemu_put_be64(f, BLK_MIG_FLAG_EOS); return 0; Now, we give exactly the same error, just that we return the error directly from blk_mig_save_dirty_block() instead of storing it on QEMUFile and getting it through qemu_file_get_error(). Notice that between old code and the _newer_ code, we have reverted the return value of blk_mig_save_block to make things more consistent. Later, Juan.
Il 24/09/2012 20:24, Juan Quintela ha scritto: > Now, we give exactly the same error, just that we return the error > directly from blk_mig_save_dirty_block() instead of storing it on > QEMUFile and getting it through qemu_file_get_error(). Right you are. Apart from the do/while, Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
diff --git a/block-migration.c b/block-migration.c index 565628f..9f94733 100644 --- a/block-migration.c +++ b/block-migration.c @@ -423,10 +423,9 @@ static int mig_save_device_dirty(QEMUFile *f, BlkMigDevState *bmds, error: DPRINTF("Error reading sector %" PRId64 "\n", sector); - qemu_file_set_error(f, ret); g_free(blk->buf); g_free(blk); - return 0; + return ret; } /* return value: @@ -440,7 +439,7 @@ static int blk_mig_save_dirty_block(QEMUFile *f, int is_async) QSIMPLEQ_FOREACH(bmds, &block_mig_state.bmds_list, entry) { ret = mig_save_device_dirty(f, bmds, is_async); - if (ret == 0) { + if (ret <= 0) { break; } } @@ -598,12 +597,17 @@ static int block_save_iterate(QEMUFile *f, void *opaque) block_mig_state.bulk_completed = 1; } } else { - if (blk_mig_save_dirty_block(f, 1) != 0) { + ret = blk_mig_save_dirty_block(f, 1); + if (ret != 0) { /* no more dirty blocks */ break; } } } + if (ret) { + blk_mig_cleanup(); + return ret; + } ret = flush_blks(f); if (ret) { @@ -635,18 +639,15 @@ static int block_save_complete(QEMUFile *f, void *opaque) all async read completed */ assert(block_mig_state.submitted == 0); - while (blk_mig_save_dirty_block(f, 0) == 0) { + while ((ret = blk_mig_save_dirty_block(f, 0)) == 0) { /* Do nothing */ } blk_mig_cleanup(); - - /* report completion */ - qemu_put_be64(f, (100 << BDRV_SECTOR_BITS) | BLK_MIG_FLAG_PROGRESS); - - ret = qemu_file_get_error(f); if (ret) { return ret; } + /* report completion */ + qemu_put_be64(f, (100 << BDRV_SECTOR_BITS) | BLK_MIG_FLAG_PROGRESS); DPRINTF("Block migration completed\n");
Signed-off-by: Juan Quintela <quintela@redhat.com> --- block-migration.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-)