Re: [PATCH v6 02/10] migration: stop returning errno from load_snapshot()

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 10/8/20 10:49 AM, Daniel P. Berrangé wrote:
>> None of the callers care about the errno value since there is a full
>> Error object populated. This gives consistency with save_snapshot()
>> which already just returns -1.
>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>> ---
>>   migration/savevm.c | 15 +++++++--------
>>   1 file changed, 7 insertions(+), 8 deletions(-)
>> 
>
>> @@ -2892,11 +2892,11 @@ int load_snapshot(const char *name, Error **errp)
>>       ret = bdrv_snapshot_find(bs_vm_state, &sn, name);
>>       aio_context_release(aio_context);
>>       if (ret < 0) {
>> -        return ret;
>> +        return -1;
>>       } else if (sn.vm_state_size == 0) {
>>           error_setg(errp, "This is a disk-only snapshot. Revert to it "
>>                      " offline using qemu-img");
>
> While you are here, let's fix the double space in the error message.

The message should be rephrased, because

 * The resulting message should be a single phrase, with no newline or
 * trailing punctuation.

This is from error_setg()'s contract.

Two obvious ways:

1. Use error_append_hint() for the "what you should do" part.

2. Replace '.' by ';' and call it a day.