[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/5] migration: split hmp_savevm to do_savevm an
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH 1/5] migration: split hmp_savevm to do_savevm and hmp_savevm wrapper |
Date: |
Wed, 23 Dec 2015 14:27:22 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 12/04/2015 07:44 AM, Denis V. Lunev wrote:
> This would be useful in the next step when QMP version of this call will
> be introduced.
>
> Signed-off-by: Denis V. Lunev <address@hidden>
> Reviewed-by: Juan Quintela <address@hidden>
> CC: Amit Shah <address@hidden>
> CC: Markus Armbruster <address@hidden>
> CC: Eric Blake <address@hidden>
> ---
> migration/savevm.c | 38 +++++++++++++++++++++++---------------
> 1 file changed, 23 insertions(+), 15 deletions(-)
>
> @@ -1915,28 +1915,27 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
> uint64_t vm_state_size;
> qemu_timeval tv;
> struct tm tm;
> - const char *name = qdict_get_try_str(qdict, "name");
> Error *local_err = NULL;
> AioContext *aio_context;
>
> if (!bdrv_all_can_snapshot(&bs)) {
> - monitor_printf(mon, "Device '%s' is writable but does not "
> - "support snapshots.\n", bdrv_get_device_name(bs));
> + error_setg(errp,
> + "Device '%s' is writable but does not support snapshots.",
No trailing '.' in error_setg() calls.
> + bdrv_get_device_name(bs));
> return;
> }
>
> /* Delete old snapshots of the same name */
> if (name && bdrv_all_delete_snapshot(name, &bs1, &local_err) < 0) {
> - monitor_printf(mon,
> - "Error while deleting snapshot on device '%s': %s\n",
> - bdrv_get_device_name(bs1),
> error_get_pretty(local_err));
> + error_setg(errp, "Error while deleting snapshot on device '%s': %s",
> + bdrv_get_device_name(bs1), error_get_pretty(local_err));
Markus' series to add a prefixing notation would be better to use here
(although I didn't check if he caught this one in that series already):
https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg03495.html
>
> +void hmp_savevm(Monitor *mon, const QDict *qdict)
> +{
> + Error *local_err = NULL;
> +
> + do_savevm(qdict_get_try_str(qdict, "name"), &local_err);
> +
> + if (local_err != NULL) {
I would have just written 'if (local_err) {'; but that's minor style.
Looks like a clean refactoring, other than the nit about the trailing
'.', so with that fixed:
Reviewed-by: Eric Blake <address@hidden>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH v2 for 2.6 0/5] QMP wrappers for VM snapshot operations, Denis V. Lunev, 2015/12/04
- [Qemu-devel] [PATCH 2/5] qmp: create qmp_savevm command, Denis V. Lunev, 2015/12/04
- [Qemu-devel] [PATCH 1/5] migration: split hmp_savevm to do_savevm and hmp_savevm wrapper, Denis V. Lunev, 2015/12/04
- Re: [Qemu-devel] [PATCH 1/5] migration: split hmp_savevm to do_savevm and hmp_savevm wrapper,
Eric Blake <=
- [Qemu-devel] [PATCH 3/5] qmp: create qmp_delvm command, Denis V. Lunev, 2015/12/04
- [Qemu-devel] [PATCH 4/5] migration: improve error reporting for load_vmstate, Denis V. Lunev, 2015/12/04
- [Qemu-devel] [PATCH 5/5] qmp: create QMP implementation of loadvm command, Denis V. Lunev, 2015/12/04
- Re: [Qemu-devel] [PATCH v2 for 2.6 0/5] QMP wrappers for VM snapshot operations, Denis V. Lunev, 2015/12/11