1. Patchew
  2. QEMU
  3. [PATCH v2 0/3] migration: Make save/load_snapshot() return boolean
  4. View patch

[PATCH v2 2/3] migration: Make save_snapshot() return bool, not 0/-1

Philippe Mathieu-Daudé posted 3 patches 5 years, 3 months ago
[PATCH v2 2/3] migration: Make save_snapshot() return bool, not 0/-1
Posted by Philippe Mathieu-Daudé 5 years, 3 months ago 
Just for consistency, following the example documented since
commit e3fe3988d7 ("error: Document Error API usage rules"),
return a boolean value indicating an error is set or not.

Acked-by: Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/migration/snapshot.h |  9 ++++++++-
 migration/savevm.c           | 16 ++++++++--------
 replay/replay-debugging.c    |  2 +-
 replay/replay-snapshot.c     |  2 +-
 4 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/include/migration/snapshot.h b/include/migration/snapshot.h
index c85b6ec75b7..a40c34307b5 100644
--- a/include/migration/snapshot.h
+++ b/include/migration/snapshot.h
@@ -15,7 +15,14 @@
 #ifndef QEMU_MIGRATION_SNAPSHOT_H
 #define QEMU_MIGRATION_SNAPSHOT_H
 
-int save_snapshot(const char *name, Error **errp);
+/**
+ * save_snapshot: Save a snapshot.
+ * @name: path to snapshot
+ * @errp: pointer to error object
+ * On success, return %true.
+ * On failure, store an error through @errp and return %false.
+ */
+bool save_snapshot(const char *name, Error **errp);
 int load_snapshot(const char *name, Error **errp);
 
 #endif
diff --git a/migration/savevm.c b/migration/savevm.c
index a52da440f4b..fd2e5e8b663 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2658,7 +2658,7 @@ int qemu_load_device_state(QEMUFile *f)
     return 0;
 }
 
-int save_snapshot(const char *name, Error **errp)
+bool save_snapshot(const char *name, Error **errp)
 {
     BlockDriverState *bs;
     QEMUSnapshotInfo sn1, *sn = &sn1, old_sn1, *old_sn = &old_sn1;
@@ -2671,29 +2671,29 @@ int save_snapshot(const char *name, Error **errp)
     AioContext *aio_context;
 
     if (migration_is_blocked(errp)) {
-        return ret;
+        return false;
     }
 
     if (!replay_can_snapshot()) {
         error_setg(errp, "Record/replay does not allow making snapshot "
                    "right now. Try once more later.");
-        return ret;
+        return false;
     }
 
     if (!bdrv_all_can_snapshot(errp)) {
-        return ret;
+        return false;
     }
 
     /* Delete old snapshots of the same name */
     if (name) {
         if (bdrv_all_delete_snapshot(name, errp) < 0) {
-            return ret;
+            return false;
         }
     }
 
     bs = bdrv_all_find_vmstate_bs(errp);
     if (bs == NULL) {
-        return ret;
+        return false;
     }
     aio_context = bdrv_get_aio_context(bs);
 
@@ -2702,7 +2702,7 @@ int save_snapshot(const char *name, Error **errp)
     ret = global_state_store();
     if (ret) {
         error_setg(errp, "Error saving global state");
-        return ret;
+        return false;
     }
     vm_stop(RUN_STATE_SAVE_VM);
 
@@ -2779,7 +2779,7 @@ int save_snapshot(const char *name, Error **errp)
     if (saved_vm_running) {
         vm_start();
     }
-    return ret;
+    return ret == 0;
 }
 
 void qmp_xen_save_devices_state(const char *filename, bool has_live, bool live,
diff --git a/replay/replay-debugging.c b/replay/replay-debugging.c
index 8785489c02d..5458a73fced 100644
--- a/replay/replay-debugging.c
+++ b/replay/replay-debugging.c
@@ -327,7 +327,7 @@ void replay_gdb_attached(void)
      */
     if (replay_mode == REPLAY_MODE_PLAY
         && !replay_snapshot) {
-        if (save_snapshot("start_debugging", NULL) != 0) {
+        if (!save_snapshot("start_debugging", NULL)) {
             /* Can't create the snapshot. Continue conventional debugging. */
         }
     }
diff --git a/replay/replay-snapshot.c b/replay/replay-snapshot.c
index e26fa4c892e..4f2560d1568 100644
--- a/replay/replay-snapshot.c
+++ b/replay/replay-snapshot.c
@@ -77,7 +77,7 @@ void replay_vmstate_init(void)
 
     if (replay_snapshot) {
         if (replay_mode == REPLAY_MODE_RECORD) {
-            if (save_snapshot(replay_snapshot, &err) != 0) {
+            if (!save_snapshot(replay_snapshot, &err)) {
                 error_report_err(err);
                 error_report("Could not create snapshot for icount record");
                 exit(1);
-- 
2.26.2
Re: [PATCH v2 2/3] migration: Make save_snapshot() return bool, not 0/-1
Posted by Daniel P. Berrangé 5 years, 3 months ago 
On Mon, Oct 12, 2020 at 02:27:42PM +0200, Philippe Mathieu-Daudé wrote:
> Just for consistency, following the example documented since
> commit e3fe3988d7 ("error: Document Error API usage rules"),
> return a boolean value indicating an error is set or not.
> 
> Acked-by: Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  include/migration/snapshot.h |  9 ++++++++-
>  migration/savevm.c           | 16 ++++++++--------
>  replay/replay-debugging.c    |  2 +-
>  replay/replay-snapshot.c     |  2 +-
>  4 files changed, 18 insertions(+), 11 deletions(-)
> 
> diff --git a/include/migration/snapshot.h b/include/migration/snapshot.h
> index c85b6ec75b7..a40c34307b5 100644
> --- a/include/migration/snapshot.h
> +++ b/include/migration/snapshot.h
> @@ -15,7 +15,14 @@
>  #ifndef QEMU_MIGRATION_SNAPSHOT_H
>  #define QEMU_MIGRATION_SNAPSHOT_H
>  
> -int save_snapshot(const char *name, Error **errp);
> +/**
> + * save_snapshot: Save a snapshot.
> + * @name: path to snapshot

This isn't a path, it is a name.

I'll apply this change:

  - * save_snapshot: Save a snapshot.
  - * @name: path to snapshot
  + * save_snapshot: Save an internal snapshot.
  + * @name: name of internal snapshot



Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [PATCH v2 2/3] migration: Make save_snapshot() return bool, not 0/-1
Posted by Philippe Mathieu-Daudé 5 years, 3 months ago 
On 10/21/20 11:56 AM, Daniel P. Berrangé wrote:
> On Mon, Oct 12, 2020 at 02:27:42PM +0200, Philippe Mathieu-Daudé wrote:
>> Just for consistency, following the example documented since
>> commit e3fe3988d7 ("error: Document Error API usage rules"),
>> return a boolean value indicating an error is set or not.
>>
>> Acked-by: Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>   include/migration/snapshot.h |  9 ++++++++-
>>   migration/savevm.c           | 16 ++++++++--------
>>   replay/replay-debugging.c    |  2 +-
>>   replay/replay-snapshot.c     |  2 +-
>>   4 files changed, 18 insertions(+), 11 deletions(-)
>>
>> diff --git a/include/migration/snapshot.h b/include/migration/snapshot.h
>> index c85b6ec75b7..a40c34307b5 100644
>> --- a/include/migration/snapshot.h
>> +++ b/include/migration/snapshot.h
>> @@ -15,7 +15,14 @@
>>   #ifndef QEMU_MIGRATION_SNAPSHOT_H
>>   #define QEMU_MIGRATION_SNAPSHOT_H
>>   
>> -int save_snapshot(const char *name, Error **errp);
>> +/**
>> + * save_snapshot: Save a snapshot.
>> + * @name: path to snapshot
> 
> This isn't a path, it is a name.
> 
> I'll apply this change:
> 
>    - * save_snapshot: Save a snapshot.
>    - * @name: path to snapshot
>    + * save_snapshot: Save an internal snapshot.
>    + * @name: name of internal snapshot

Thanks!

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.