[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v9 02/11] migration: Make save_snapshot() return bool, not 0/
From: |
Eric Blake |
Subject: |
Re: [PATCH v9 02/11] migration: Make save_snapshot() return bool, not 0/-1 |
Date: |
Wed, 20 Jan 2021 12:48:55 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0 |
On 1/20/21 12:46 PM, Eric Blake wrote:
> On 1/20/21 4:44 AM, Daniel P. Berrangé wrote:
>> From: Philippe Mathieu-Daudé <philmd@redhat.com>
>>
>> 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(-)
>
>
>> +++ b/migration/savevm.c
>> @@ -2729,7 +2729,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)
>> {
>
> Missing a 'return -1' that must be changed to 'return false' (see patch
> 11; if not fixed here, you have a window where you are inadvertently
> returning true on failure).
>
Correction, it was patch 6/11 that introduced the 'return -1' that then
got fixed in 11/11; patch 2/11 by itself is okay.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
- [PATCH v9 00/11] migration: bring improved savevm/loadvm/delvm to QMP, Daniel P . Berrangé, 2021/01/20
- [PATCH v9 01/11] block: push error reporting into bdrv_all_*_snapshot functions, Daniel P . Berrangé, 2021/01/20
- [PATCH v9 02/11] migration: Make save_snapshot() return bool, not 0/-1, Daniel P . Berrangé, 2021/01/20
- [PATCH v9 03/11] migration: stop returning errno from load_snapshot(), Daniel P . Berrangé, 2021/01/20
- [PATCH v9 04/11] block: add ability to specify list of blockdevs during snapshot, Daniel P . Berrangé, 2021/01/20
- [PATCH v9 07/11] migration: control whether snapshots are ovewritten, Daniel P . Berrangé, 2021/01/20
- [PATCH v9 06/11] block: rename and alter bdrv_all_find_snapshot semantics, Daniel P . Berrangé, 2021/01/20
- [PATCH v9 05/11] block: allow specifying name of block device for vmstate storage, Daniel P . Berrangé, 2021/01/20
- [PATCH v9 08/11] migration: wire up support for snapshot device selection, Daniel P . Berrangé, 2021/01/20
- [PATCH v9 09/11] migration: introduce a delete_snapshot wrapper, Daniel P . Berrangé, 2021/01/20