| Message ID | 1449240275-26196-5-git-send-email-den@openvz.org |
|---|---|
| State | New |
| Headers | show |
On 12/04/2015 07:44 AM, Denis V. Lunev wrote: > The patch adds Error ** parameter to load_vmstate call and fills error > inside. The caller after that properly reports error either through > monitor or via local stderr facility during VM start. > > This helper will be useful too for qmp_loadvm implementation. > > Signed-off-by: Denis V. Lunev <den@openvz.org> > CC: Juan Quintela <quintela@redhat.com> > CC: Amit Shah <amit.shah@redhat.com> > CC: Markus Armbruster <armbru@redhat.com> > CC: Eric Blake <eblake@redhat.com> > --- > include/sysemu/sysemu.h | 2 +- > migration/savevm.c | 28 ++++++++++++++++------------ > monitor.c | 6 +++++- > vl.c | 4 +++- > 4 files changed, 25 insertions(+), 15 deletions(-) > > -int load_vmstate(const char *name) > +int load_vmstate(const char *name, Error **errp) > { > BlockDriverState *bs, *bs_vm_state; > QEMUSnapshotInfo sn; > @@ -2035,20 +2035,22 @@ int load_vmstate(const char *name) > AioContext *aio_context; > > if (!bdrv_all_can_snapshot(&bs)) { > - error_report("Device '%s' is writable but does not support snapshots.", > - bdrv_get_device_name(bs)); > + error_setg(errp, > + "Device '%s' is writable but does not support snapshots.", No trailing '.' > @@ -2058,10 +2060,11 @@ int load_vmstate(const char *name) > ret = bdrv_snapshot_find(bs_vm_state, &sn, name); > aio_context_release(aio_context); > if (ret < 0) { > + error_setg_errno(errp, ret, "Snapshot '%s' not found", name); You need '-ret', since error_setg_errno() expects positive errno values. [maybe, for convenience, we should teach error_setg_errno() to handle both -EINVAL and EINVAL identically - but it would need good justification and would touch a lot of the tree, so if we do it, it would be a separate series] > return ret; > } else if (sn.vm_state_size == 0) { > - error_report("This is a disk-only snapshot. Revert to it offline " > - "using qemu-img."); > + error_setg(errp, "This is a disk-only snapshot. Revert to it offline " > + "using qemu-img."); According to Markus' recent cleanups, error_setg() should be a single phrase, not two sentences. You'll want the second sentence to be added with error_append_hint(): https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg03508.html > return -EINVAL; > } > > @@ -2070,15 +2073,16 @@ int load_vmstate(const char *name) > > 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)); > + error_setg_errno(errp, ret, -ret again > + "Error while activating snapshot '%s' on '%s'", > + name, bdrv_get_device_name(bs)); but thanks for getting rid of the weird 'error -5 while...' in the message :) > @@ -2092,7 +2096,7 @@ int load_vmstate(const char *name) > > migration_incoming_state_destroy(); > if (ret < 0) { > - error_report("Error %d while loading VM state", ret); > + error_setg_errno(errp, ret, "Error while loading VM state"); -ret again > +++ b/monitor.c > @@ -1739,10 +1739,14 @@ static void hmp_loadvm(Monitor *mon, const QDict *qdict) > { > int saved_vm_running = runstate_is_running(); > const char *name = qdict_get_str(qdict, "name"); > + Error *local_err = NULL; > > vm_stop(RUN_STATE_RESTORE_VM); > > - if (load_vmstate(name) == 0 && saved_vm_running) { > + if (load_vmstate(name, &local_err) < 0) { > + error_report_err(local_err); > + } > + if (saved_vm_running) { > vm_start(); Logic bug. You blindly fall through, and could end up attempting vm_start() even after an error, where previously it was not possible. You probably meant '} else if (saved_vm_running) {'.
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h index 3bb8897..d9ccf45 100644 --- a/include/sysemu/sysemu.h +++ b/include/sysemu/sysemu.h @@ -78,7 +78,7 @@ void qemu_remove_exit_notifier(Notifier *notify); void qemu_add_machine_init_done_notifier(Notifier *notify); void hmp_savevm(Monitor *mon, const QDict *qdict); -int load_vmstate(const char *name); +int load_vmstate(const char *name, Error **errp); void hmp_delvm(Monitor *mon, const QDict *qdict); void hmp_info_snapshots(Monitor *mon, const QDict *qdict); diff --git a/migration/savevm.c b/migration/savevm.c index f87a061..7846437 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -2026,7 +2026,7 @@ void qmp_xen_save_devices_state(const char *filename, Error **errp) } } -int load_vmstate(const char *name) +int load_vmstate(const char *name, Error **errp) { BlockDriverState *bs, *bs_vm_state; QEMUSnapshotInfo sn; @@ -2035,20 +2035,22 @@ int load_vmstate(const char *name) AioContext *aio_context; if (!bdrv_all_can_snapshot(&bs)) { - error_report("Device '%s' is writable but does not support snapshots.", - bdrv_get_device_name(bs)); + error_setg(errp, + "Device '%s' is writable but does not support snapshots.", + bdrv_get_device_name(bs)); return -ENOTSUP; } ret = bdrv_all_find_snapshot(name, &bs); if (ret < 0) { - error_report("Device '%s' does not have the requested snapshot '%s'", - bdrv_get_device_name(bs), name); + error_setg(errp, + "Device '%s' does not have the requested snapshot '%s'", + bdrv_get_device_name(bs), name); return ret; } bs_vm_state = bdrv_all_find_vmstate_bs(); if (!bs_vm_state) { - error_report("No block device supports snapshots"); + error_setg(errp, "No block device supports snapshots"); return -ENOTSUP; } aio_context = bdrv_get_aio_context(bs_vm_state); @@ -2058,10 +2060,11 @@ int load_vmstate(const char *name) ret = bdrv_snapshot_find(bs_vm_state, &sn, name); aio_context_release(aio_context); if (ret < 0) { + error_setg_errno(errp, ret, "Snapshot '%s' not found", name); return ret; } else if (sn.vm_state_size == 0) { - error_report("This is a disk-only snapshot. Revert to it offline " - "using qemu-img."); + error_setg(errp, "This is a disk-only snapshot. Revert to it offline " + "using qemu-img."); return -EINVAL; } @@ -2070,15 +2073,16 @@ int load_vmstate(const char *name) 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)); + error_setg_errno(errp, ret, + "Error while activating snapshot '%s' on '%s'", + name, bdrv_get_device_name(bs)); return ret; } /* restore the VM state */ f = qemu_fopen_bdrv(bs_vm_state, 0); if (!f) { - error_report("Could not open VM state file"); + error_setg(errp, "Could not open VM state file"); return -EINVAL; } @@ -2092,7 +2096,7 @@ int load_vmstate(const char *name) migration_incoming_state_destroy(); if (ret < 0) { - error_report("Error %d while loading VM state", ret); + error_setg_errno(errp, ret, "Error while loading VM state"); return ret; } diff --git a/monitor.c b/monitor.c index 9a35d72..3857724 100644 --- a/monitor.c +++ b/monitor.c @@ -1739,10 +1739,14 @@ static void hmp_loadvm(Monitor *mon, const QDict *qdict) { int saved_vm_running = runstate_is_running(); const char *name = qdict_get_str(qdict, "name"); + Error *local_err = NULL; vm_stop(RUN_STATE_RESTORE_VM); - if (load_vmstate(name) == 0 && saved_vm_running) { + if (load_vmstate(name, &local_err) < 0) { + error_report_err(local_err); + } + if (saved_vm_running) { vm_start(); } } diff --git a/vl.c b/vl.c index 4211ff1..ee34edb 100644 --- a/vl.c +++ b/vl.c @@ -4648,8 +4648,10 @@ int main(int argc, char **argv, char **envp) qemu_system_reset(VMRESET_SILENT); register_global_state(); if (loadvm) { - if (load_vmstate(loadvm) < 0) { + Error *local_err = NULL; + if (load_vmstate(loadvm, &local_err) < 0) { autostart = 0; + error_report_err(local_err); } }
The patch adds Error ** parameter to load_vmstate call and fills error inside. The caller after that properly reports error either through monitor or via local stderr facility during VM start. This helper will be useful too for qmp_loadvm implementation. Signed-off-by: Denis V. Lunev <den@openvz.org> CC: Juan Quintela <quintela@redhat.com> CC: Amit Shah <amit.shah@redhat.com> CC: Markus Armbruster <armbru@redhat.com> CC: Eric Blake <eblake@redhat.com> --- include/sysemu/sysemu.h | 2 +- migration/savevm.c | 28 ++++++++++++++++------------ monitor.c | 6 +++++- vl.c | 4 +++- 4 files changed, 25 insertions(+), 15 deletions(-)