Patchwork [for-1.4,stable,2/3] block-migration: fix blk_mig_save_dirty_block() return value checking

login
register
mail settings
Submitter Stefan Hajnoczi
Date Feb. 9, 2013, 6:01 p.m.
Message ID <1360432919-1379-3-git-send-email-stefanha@redhat.com>
Download mbox | patch
Permalink /patch/219426/
State New
Headers show

Comments

Stefan Hajnoczi - Feb. 9, 2013, 6:01 p.m.
Commit 43be3a25c931a7f61a76fbfc9d35584cbfc5fb58 changed the
blk_mig_save_dirty_block() return code handling.  The function's doc
comment says:

  /* return value:
   * 0: too much data for max_downtime
   * 1: few enough data for max_downtime
   */

Because of the 1 return value, callers must check for ret < 0 instead of
just:

  if (ret) { ... }

We do not want to bail when 1 is returned, only on error.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block-migration.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Paolo Bonzini - Feb. 11, 2013, 10:17 a.m.
Il 09/02/2013 19:01, Stefan Hajnoczi ha scritto:
> Commit 43be3a25c931a7f61a76fbfc9d35584cbfc5fb58 changed the
> blk_mig_save_dirty_block() return code handling.  The function's doc
> comment says:
> 
>   /* return value:
>    * 0: too much data for max_downtime
>    * 1: few enough data for max_downtime
>    */
> 
> Because of the 1 return value, callers must check for ret < 0 instead of
> just:
> 
>   if (ret) { ... }
> 
> We do not want to bail when 1 is returned, only on error.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  block-migration.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/block-migration.c b/block-migration.c
> index 573319a..a91d96b 100644
> --- a/block-migration.c
> +++ b/block-migration.c
> @@ -569,7 +569,7 @@ static int block_save_iterate(QEMUFile *f, void *opaque)
>              }
>          }
>      }
> -    if (ret) {
> +    if (ret < 0) {
>          blk_mig_cleanup();
>          return ret;
>      }
> @@ -609,7 +609,7 @@ static int block_save_complete(QEMUFile *f, void *opaque)
>      } while (ret == 0);
>  
>      blk_mig_cleanup();
> -    if (ret) {
> +    if (ret < 0) {
>          return ret;
>      }
>      /* report completion */
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Juan Quintela - Feb. 11, 2013, 12:56 p.m.
Stefan Hajnoczi <stefanha@redhat.com> wrote:
> Commit 43be3a25c931a7f61a76fbfc9d35584cbfc5fb58 changed the
> blk_mig_save_dirty_block() return code handling.  The function's doc
> comment says:
>
>   /* return value:
>    * 0: too much data for max_downtime
>    * 1: few enough data for max_downtime
>    */
>
> Because of the 1 return value, callers must check for ret < 0 instead of
> just:
>
>   if (ret) { ... }
>
> We do not want to bail when 1 is returned, only on error.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

Patch

diff --git a/block-migration.c b/block-migration.c
index 573319a..a91d96b 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -569,7 +569,7 @@  static int block_save_iterate(QEMUFile *f, void *opaque)
             }
         }
     }
-    if (ret) {
+    if (ret < 0) {
         blk_mig_cleanup();
         return ret;
     }
@@ -609,7 +609,7 @@  static int block_save_complete(QEMUFile *f, void *opaque)
     } while (ret == 0);
 
     blk_mig_cleanup();
-    if (ret) {
+    if (ret < 0) {
         return ret;
     }
     /* report completion */