[Qemu-devel] [PATCH for-2.11 2/3] block: Add errp to bdrv_all_goto_snapshot()

Kevin Wolf posted 3 patches 8 years, 2 months ago
[Qemu-devel] [PATCH for-2.11 2/3] block: Add errp to bdrv_all_goto_snapshot()
Posted by Kevin Wolf 8 years, 2 months ago
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 include/block/snapshot.h |  3 ++-
 block/snapshot.c         | 11 ++++++-----
 migration/savevm.c       |  6 +++---
 3 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/include/block/snapshot.h b/include/block/snapshot.h
index aeb80405e8..9407799941 100644
--- a/include/block/snapshot.h
+++ b/include/block/snapshot.h
@@ -84,7 +84,8 @@ int bdrv_snapshot_load_tmp_by_id_or_name(BlockDriverState *bs,
 bool bdrv_all_can_snapshot(BlockDriverState **first_bad_bs);
 int bdrv_all_delete_snapshot(const char *name, BlockDriverState **first_bsd_bs,
                              Error **err);
-int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bsd_bs);
+int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs,
+                           Error **errp);
 int bdrv_all_find_snapshot(const char *name, BlockDriverState **first_bad_bs);
 int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn,
                              BlockDriverState *vm_state_bs,
diff --git a/block/snapshot.c b/block/snapshot.c
index 9c941e638d..13ec3b1c8c 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -467,9 +467,10 @@ fail:
 }
 
 
-int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs)
+int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs,
+                           Error **errp)
 {
-    int err = 0;
+    int ret = 0;
     BlockDriverState *bs;
     BdrvNextIterator it;
 
@@ -478,10 +479,10 @@ int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs)
 
         aio_context_acquire(ctx);
         if (bdrv_can_snapshot(bs)) {
-            err = bdrv_snapshot_goto(bs, name, NULL);
+            ret = bdrv_snapshot_goto(bs, name, errp);
         }
         aio_context_release(ctx);
-        if (err < 0) {
+        if (ret < 0) {
             bdrv_next_cleanup(&it);
             goto fail;
         }
@@ -489,7 +490,7 @@ int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs)
 
 fail:
     *first_bad_bs = bs;
-    return err;
+    return ret;
 }
 
 int bdrv_all_find_snapshot(const char *name, BlockDriverState **first_bad_bs)
diff --git a/migration/savevm.c b/migration/savevm.c
index 4a88228614..192f2d82cd 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2346,10 +2346,10 @@ int load_snapshot(const char *name, Error **errp)
     /* Flush all IO requests so they don't interfere with the new state.  */
     bdrv_drain_all_begin();
 
-    ret = bdrv_all_goto_snapshot(name, &bs);
+    ret = bdrv_all_goto_snapshot(name, &bs, errp);
     if (ret < 0) {
-        error_setg(errp, "Error %d while activating snapshot '%s' on '%s'",
-                     ret, name, bdrv_get_device_name(bs));
+        error_prepend(errp, "Could not load snapshot '%s' on '%s': ",
+                      name, bdrv_get_device_name(bs));
         goto err_drain;
     }
 
-- 
2.13.6


Re: [Qemu-devel] [PATCH for-2.11 2/3] block: Add errp to bdrv_all_goto_snapshot()
Posted by Vladimir Sementsov-Ogievskiy 8 years, 2 months ago
20.11.2017 17:50, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   include/block/snapshot.h |  3 ++-
>   block/snapshot.c         | 11 ++++++-----
>   migration/savevm.c       |  6 +++---
>   3 files changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/include/block/snapshot.h b/include/block/snapshot.h
> index aeb80405e8..9407799941 100644
> --- a/include/block/snapshot.h
> +++ b/include/block/snapshot.h
> @@ -84,7 +84,8 @@ int bdrv_snapshot_load_tmp_by_id_or_name(BlockDriverState *bs,
>   bool bdrv_all_can_snapshot(BlockDriverState **first_bad_bs);
>   int bdrv_all_delete_snapshot(const char *name, BlockDriverState **first_bsd_bs,
>                                Error **err);
> -int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bsd_bs);
> +int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs,
> +                           Error **errp);
>   int bdrv_all_find_snapshot(const char *name, BlockDriverState **first_bad_bs);
>   int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn,
>                                BlockDriverState *vm_state_bs,
> diff --git a/block/snapshot.c b/block/snapshot.c
> index 9c941e638d..13ec3b1c8c 100644
> --- a/block/snapshot.c
> +++ b/block/snapshot.c
> @@ -467,9 +467,10 @@ fail:
>   }
>   
>   
> -int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs)
> +int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs,
> +                           Error **errp)
>   {
> -    int err = 0;
> +    int ret = 0;
>       BlockDriverState *bs;
>       BdrvNextIterator it;
>   
> @@ -478,10 +479,10 @@ int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs)
>   
>           aio_context_acquire(ctx);
>           if (bdrv_can_snapshot(bs)) {
> -            err = bdrv_snapshot_goto(bs, name, NULL);
> +            ret = bdrv_snapshot_goto(bs, name, errp);
>           }
>           aio_context_release(ctx);
> -        if (err < 0) {
> +        if (ret < 0) {
>               bdrv_next_cleanup(&it);
>               goto fail;
>           }
> @@ -489,7 +490,7 @@ int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs)
>   
>   fail:
>       *first_bad_bs = bs;
> -    return err;
> +    return ret;
>   }
>   
>   int bdrv_all_find_snapshot(const char *name, BlockDriverState **first_bad_bs)
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 4a88228614..192f2d82cd 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -2346,10 +2346,10 @@ int load_snapshot(const char *name, Error **errp)
>       /* Flush all IO requests so they don't interfere with the new state.  */
>       bdrv_drain_all_begin();
>   
> -    ret = bdrv_all_goto_snapshot(name, &bs);
> +    ret = bdrv_all_goto_snapshot(name, &bs, errp);
>       if (ret < 0) {
> -        error_setg(errp, "Error %d while activating snapshot '%s' on '%s'",
> -                     ret, name, bdrv_get_device_name(bs));
> +        error_prepend(errp, "Could not load snapshot '%s' on '%s': ",
> +                      name, bdrv_get_device_name(bs));

possible resulting msg: "Could not load snapshot 'snap0' on 'dev0': 
Failed to load snapshot". a bit weird. with this fixed or not:

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

>           goto err_drain;
>       }
>   


-- 
Best regards,
Vladimir