| Message ID | 20201008155001.3357288-3-berrange@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | migration: bring improved savevm/loadvm/delvm to QMP | expand |
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.
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.
diff --git a/migration/savevm.c b/migration/savevm.c index a52da440f4..87eaa07553 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -2874,16 +2874,16 @@ int load_snapshot(const char *name, Error **errp) MigrationIncomingState *mis = migration_incoming_get_current(); if (!bdrv_all_can_snapshot(errp)) { - return -ENOTSUP; + return -1; } ret = bdrv_all_find_snapshot(name, errp); if (ret < 0) { - return ret; + return -1; } bs_vm_state = bdrv_all_find_vmstate_bs(errp); if (!bs_vm_state) { - return -ENOTSUP; + return -1; } aio_context = bdrv_get_aio_context(bs_vm_state); @@ -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"); - return -EINVAL; + return -1; } /* @@ -2917,7 +2917,6 @@ int load_snapshot(const char *name, Error **errp) f = qemu_fopen_bdrv(bs_vm_state, 0); if (!f) { error_setg(errp, "Could not open VM state file"); - ret = -EINVAL; goto err_drain; } @@ -2933,14 +2932,14 @@ int load_snapshot(const char *name, Error **errp) if (ret < 0) { error_setg(errp, "Error %d while loading VM state", ret); - return ret; + return -1; } return 0; err_drain: bdrv_drain_all_end(); - return ret; + return -1; } void vmstate_register_ram(MemoryRegion *mr, DeviceState *dev)