diff mbox

[04/10] snapshot: create bdrv_all_goto_snapshot helper

Message ID 1447165535-31263-5-git-send-email-den@openvz.org
State New
Headers show

Commit Message

Denis V. Lunev Nov. 10, 2015, 2:25 p.m. UTC
to switch to snapshot on all loaded block drivers.

The patch also ensures proper locking.

Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
CC: Juan Quintela <quintela@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
---
 block/snapshot.c         | 20 ++++++++++++++++++++
 include/block/snapshot.h |  1 +
 migration/savevm.c       | 15 +++++----------
 3 files changed, 26 insertions(+), 10 deletions(-)

Comments

Fam Zheng Nov. 12, 2015, 8:38 a.m. UTC | #1
On Tue, 11/10 17:25, Denis V. Lunev wrote:
> to switch to snapshot on all loaded block drivers.
> 
> The patch also ensures proper locking.
> 
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> Reviewed-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> CC: Juan Quintela <quintela@redhat.com>
> CC: Stefan Hajnoczi <stefanha@redhat.com>
> CC: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/snapshot.c         | 20 ++++++++++++++++++++
>  include/block/snapshot.h |  1 +
>  migration/savevm.c       | 15 +++++----------
>  3 files changed, 26 insertions(+), 10 deletions(-)
> 
> diff --git a/block/snapshot.c b/block/snapshot.c
> index 61a6ad1..9f07a63 100644
> --- a/block/snapshot.c
> +++ b/block/snapshot.c
> @@ -403,3 +403,23 @@ int bdrv_all_delete_snapshot(const char *name, BlockDriverState **first_bad_bs,
>      *first_bad_bs = bs;
>      return ret;
>  }
> +
> +
> +int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs)
> +{
> +    int err = 0;
> +    BlockDriverState *bs = NULL;
> +
> +    while (err == 0 && (bs = bdrv_next(bs))) {
> +        AioContext *ctx = bdrv_get_aio_context(bs);
> +
> +        aio_context_acquire(ctx);
> +        if (bdrv_can_snapshot(bs)) {
> +            err = bdrv_snapshot_goto(bs, name);
> +        }
> +        aio_context_release(ctx);
> +    }
> +
> +    *first_bad_bs = bs;
> +    return err;
> +}
> diff --git a/include/block/snapshot.h b/include/block/snapshot.h
> index d02d2b1..0a176c7 100644
> --- a/include/block/snapshot.h
> +++ b/include/block/snapshot.h
> @@ -84,5 +84,6 @@ 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);
>  
>  #endif
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 1157a6f..d18ff13 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1425,16 +1425,11 @@ int load_vmstate(const char *name)
>      /* Flush all IO requests so they don't interfere with the new state.  */
>      bdrv_drain_all();
>  
> -    bs = NULL;
> -    while ((bs = bdrv_next(bs))) {
> -        if (bdrv_can_snapshot(bs)) {
> -            ret = bdrv_snapshot_goto(bs, name);
> -            if (ret < 0) {
> -                error_report("Error %d while activating snapshot '%s' on '%s'",
> -                             ret, name, bdrv_get_device_name(bs));
> -                return ret;
> -            }
> -        }
> +    ret = bdrv_all_goto_snapshot(name, &bs);
> +    if (ret < 0) {
> +        error_report("Error %d while activating snapshot '%s' on '%s'",
> +                     ret, name, bdrv_get_device_name(bs));
> +        return ret;

Maybe more friendlily strerror(ret)?

>      }
>  
>      /* restore the VM state */
> -- 
> 2.5.0
> 
>
Denis V. Lunev Nov. 12, 2015, 9:48 a.m. UTC | #2
On 11/12/2015 11:38 AM, Fam Zheng wrote:
> On Tue, 11/10 17:25, Denis V. Lunev wrote:
>> to switch to snapshot on all loaded block drivers.
>>
>> The patch also ensures proper locking.
>>
>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>> Reviewed-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
>> CC: Juan Quintela <quintela@redhat.com>
>> CC: Stefan Hajnoczi <stefanha@redhat.com>
>> CC: Kevin Wolf <kwolf@redhat.com>
>> ---
>>   block/snapshot.c         | 20 ++++++++++++++++++++
>>   include/block/snapshot.h |  1 +
>>   migration/savevm.c       | 15 +++++----------
>>   3 files changed, 26 insertions(+), 10 deletions(-)
>>
>> diff --git a/block/snapshot.c b/block/snapshot.c
>> index 61a6ad1..9f07a63 100644
>> --- a/block/snapshot.c
>> +++ b/block/snapshot.c
>> @@ -403,3 +403,23 @@ int bdrv_all_delete_snapshot(const char *name, BlockDriverState **first_bad_bs,
>>       *first_bad_bs = bs;
>>       return ret;
>>   }
>> +
>> +
>> +int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs)
>> +{
>> +    int err = 0;
>> +    BlockDriverState *bs = NULL;
>> +
>> +    while (err == 0 && (bs = bdrv_next(bs))) {
>> +        AioContext *ctx = bdrv_get_aio_context(bs);
>> +
>> +        aio_context_acquire(ctx);
>> +        if (bdrv_can_snapshot(bs)) {
>> +            err = bdrv_snapshot_goto(bs, name);
>> +        }
>> +        aio_context_release(ctx);
>> +    }
>> +
>> +    *first_bad_bs = bs;
>> +    return err;
>> +}
>> diff --git a/include/block/snapshot.h b/include/block/snapshot.h
>> index d02d2b1..0a176c7 100644
>> --- a/include/block/snapshot.h
>> +++ b/include/block/snapshot.h
>> @@ -84,5 +84,6 @@ 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);
>>   
>>   #endif
>> diff --git a/migration/savevm.c b/migration/savevm.c
>> index 1157a6f..d18ff13 100644
>> --- a/migration/savevm.c
>> +++ b/migration/savevm.c
>> @@ -1425,16 +1425,11 @@ int load_vmstate(const char *name)
>>       /* Flush all IO requests so they don't interfere with the new state.  */
>>       bdrv_drain_all();
>>   
>> -    bs = NULL;
>> -    while ((bs = bdrv_next(bs))) {
>> -        if (bdrv_can_snapshot(bs)) {
>> -            ret = bdrv_snapshot_goto(bs, name);
>> -            if (ret < 0) {
>> -                error_report("Error %d while activating snapshot '%s' on '%s'",
>> -                             ret, name, bdrv_get_device_name(bs));
>> -                return ret;
>> -            }
>> -        }
>> +    ret = bdrv_all_goto_snapshot(name, &bs);
>> +    if (ret < 0) {
>> +        error_report("Error %d while activating snapshot '%s' on '%s'",
>> +                     ret, name, bdrv_get_device_name(bs));
>> +        return ret;
> Maybe more friendlily strerror(ret)?
>
>>       }
>>   
>>       /* restore the VM state */
>> -- 
>> 2.5.0
>>
>>
we can. I think that this could be done for all such functions
as follow up patch. I'll do that separately and will send
with the next iteration if applicable.

Den
diff mbox

Patch

diff --git a/block/snapshot.c b/block/snapshot.c
index 61a6ad1..9f07a63 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -403,3 +403,23 @@  int bdrv_all_delete_snapshot(const char *name, BlockDriverState **first_bad_bs,
     *first_bad_bs = bs;
     return ret;
 }
+
+
+int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs)
+{
+    int err = 0;
+    BlockDriverState *bs = NULL;
+
+    while (err == 0 && (bs = bdrv_next(bs))) {
+        AioContext *ctx = bdrv_get_aio_context(bs);
+
+        aio_context_acquire(ctx);
+        if (bdrv_can_snapshot(bs)) {
+            err = bdrv_snapshot_goto(bs, name);
+        }
+        aio_context_release(ctx);
+    }
+
+    *first_bad_bs = bs;
+    return err;
+}
diff --git a/include/block/snapshot.h b/include/block/snapshot.h
index d02d2b1..0a176c7 100644
--- a/include/block/snapshot.h
+++ b/include/block/snapshot.h
@@ -84,5 +84,6 @@  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);
 
 #endif
diff --git a/migration/savevm.c b/migration/savevm.c
index 1157a6f..d18ff13 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1425,16 +1425,11 @@  int load_vmstate(const char *name)
     /* Flush all IO requests so they don't interfere with the new state.  */
     bdrv_drain_all();
 
-    bs = NULL;
-    while ((bs = bdrv_next(bs))) {
-        if (bdrv_can_snapshot(bs)) {
-            ret = bdrv_snapshot_goto(bs, name);
-            if (ret < 0) {
-                error_report("Error %d while activating snapshot '%s' on '%s'",
-                             ret, name, bdrv_get_device_name(bs));
-                return ret;
-            }
-        }
+    ret = bdrv_all_goto_snapshot(name, &bs);
+    if (ret < 0) {
+        error_report("Error %d while activating snapshot '%s' on '%s'",
+                     ret, name, bdrv_get_device_name(bs));
+        return ret;
     }
 
     /* restore the VM state */