From e26f98e2097cf17db04462e9aa2e423b93e7455c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Thu, 4 Feb 2021 12:48:23 +0000 Subject: block: push error reporting into bdrv_all_*_snapshot functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bdrv_all_*_snapshot functions return a BlockDriverState pointer for the invalid backend, which the callers then use to report an error message. In some cases multiple callers are reporting the same error message, but with slightly different text. In the future there will be more error scenarios for some of these methods, which will benefit from fine grained error message reporting. So it is helpful to push error reporting down a level. Reviewed-by: Eric Blake Signed-off-by: Daniel P. Berrangé [PMD: Initialize variables] Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20210204124834.774401-2-berrange@redhat.com> Signed-off-by: Dr. David Alan Gilbert --- block/monitor/block-hmp-cmds.c | 7 ++-- block/snapshot.c | 77 +++++++++++++++++++++--------------------- 2 files changed, 43 insertions(+), 41 deletions(-) (limited to 'block') diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c index afd75ab..9532d08 100644 --- a/block/monitor/block-hmp-cmds.c +++ b/block/monitor/block-hmp-cmds.c @@ -900,10 +900,11 @@ void hmp_info_snapshots(Monitor *mon, const QDict *qdict) ImageEntry *image_entry, *next_ie; SnapshotEntry *snapshot_entry; + Error *err = NULL; - bs = bdrv_all_find_vmstate_bs(); + bs = bdrv_all_find_vmstate_bs(&err); if (!bs) { - monitor_printf(mon, "No available block device supports snapshots\n"); + error_report_err(err); return; } aio_context = bdrv_get_aio_context(bs); @@ -953,7 +954,7 @@ void hmp_info_snapshots(Monitor *mon, const QDict *qdict) total = 0; for (i = 0; i < nb_sns; i++) { SnapshotEntry *next_sn; - if (bdrv_all_find_snapshot(sn_tab[i].name, &bs1) == 0) { + if (bdrv_all_find_snapshot(sn_tab[i].name, NULL) == 0) { global_snapshots[total] = i; total++; QTAILQ_FOREACH(image_entry, &image_list, next) { diff --git a/block/snapshot.c b/block/snapshot.c index a2bf3a5..482e3fc 100644 --- a/block/snapshot.c +++ b/block/snapshot.c @@ -462,14 +462,14 @@ static bool bdrv_all_snapshots_includes_bs(BlockDriverState *bs) * These functions will properly handle dataplane (take aio_context_acquire * when appropriate for appropriate block drivers) */ -bool bdrv_all_can_snapshot(BlockDriverState **first_bad_bs) +bool bdrv_all_can_snapshot(Error **errp) { - bool ok = true; BlockDriverState *bs; BdrvNextIterator it; for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) { AioContext *ctx = bdrv_get_aio_context(bs); + bool ok = true; aio_context_acquire(ctx); if (bdrv_all_snapshots_includes_bs(bs)) { @@ -477,26 +477,25 @@ bool bdrv_all_can_snapshot(BlockDriverState **first_bad_bs) } aio_context_release(ctx); if (!ok) { + error_setg(errp, "Device '%s' is writable but does not support " + "snapshots", bdrv_get_device_or_node_name(bs)); bdrv_next_cleanup(&it); - goto fail; + return false; } } -fail: - *first_bad_bs = bs; - return ok; + return true; } -int bdrv_all_delete_snapshot(const char *name, BlockDriverState **first_bad_bs, - Error **errp) +int bdrv_all_delete_snapshot(const char *name, Error **errp) { - int ret = 0; BlockDriverState *bs; BdrvNextIterator it; QEMUSnapshotInfo sn1, *snapshot = &sn1; for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) { AioContext *ctx = bdrv_get_aio_context(bs); + int ret = 0; aio_context_acquire(ctx); if (bdrv_all_snapshots_includes_bs(bs) && @@ -507,26 +506,25 @@ int bdrv_all_delete_snapshot(const char *name, BlockDriverState **first_bad_bs, } aio_context_release(ctx); if (ret < 0) { + error_prepend(errp, "Could not delete snapshot '%s' on '%s': ", + name, bdrv_get_device_or_node_name(bs)); bdrv_next_cleanup(&it); - goto fail; + return -1; } } -fail: - *first_bad_bs = bs; - return ret; + return 0; } -int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs, - Error **errp) +int bdrv_all_goto_snapshot(const char *name, Error **errp) { - int ret = 0; BlockDriverState *bs; BdrvNextIterator it; for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) { AioContext *ctx = bdrv_get_aio_context(bs); + int ret = 0; aio_context_acquire(ctx); if (bdrv_all_snapshots_includes_bs(bs)) { @@ -534,75 +532,75 @@ int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs, } aio_context_release(ctx); if (ret < 0) { + error_prepend(errp, "Could not load snapshot '%s' on '%s': ", + name, bdrv_get_device_or_node_name(bs)); bdrv_next_cleanup(&it); - goto fail; + return -1; } } -fail: - *first_bad_bs = bs; - return ret; + return 0; } -int bdrv_all_find_snapshot(const char *name, BlockDriverState **first_bad_bs) +int bdrv_all_find_snapshot(const char *name, Error **errp) { QEMUSnapshotInfo sn; - int err = 0; BlockDriverState *bs; BdrvNextIterator it; for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) { AioContext *ctx = bdrv_get_aio_context(bs); + int ret = 0; aio_context_acquire(ctx); if (bdrv_all_snapshots_includes_bs(bs)) { - err = bdrv_snapshot_find(bs, &sn, name); + ret = bdrv_snapshot_find(bs, &sn, name); } aio_context_release(ctx); - if (err < 0) { + if (ret < 0) { + error_setg(errp, "Could not find snapshot '%s' on '%s'", + name, bdrv_get_device_or_node_name(bs)); bdrv_next_cleanup(&it); - goto fail; + return -1; } } -fail: - *first_bad_bs = bs; - return err; + return 0; } int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn, BlockDriverState *vm_state_bs, uint64_t vm_state_size, - BlockDriverState **first_bad_bs) + Error **errp) { - int err = 0; BlockDriverState *bs; BdrvNextIterator it; for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) { AioContext *ctx = bdrv_get_aio_context(bs); + int ret = 0; aio_context_acquire(ctx); if (bs == vm_state_bs) { sn->vm_state_size = vm_state_size; - err = bdrv_snapshot_create(bs, sn); + ret = bdrv_snapshot_create(bs, sn); } else if (bdrv_all_snapshots_includes_bs(bs)) { sn->vm_state_size = 0; - err = bdrv_snapshot_create(bs, sn); + ret = bdrv_snapshot_create(bs, sn); } aio_context_release(ctx); - if (err < 0) { + if (ret < 0) { + error_setg(errp, "Could not create snapshot '%s' on '%s'", + sn->name, bdrv_get_device_or_node_name(bs)); bdrv_next_cleanup(&it); - goto fail; + return -1; } } -fail: - *first_bad_bs = bs; - return err; + return 0; } -BlockDriverState *bdrv_all_find_vmstate_bs(void) +BlockDriverState *bdrv_all_find_vmstate_bs(Error **errp) { BlockDriverState *bs; BdrvNextIterator it; @@ -620,5 +618,8 @@ BlockDriverState *bdrv_all_find_vmstate_bs(void) break; } } + if (!bs) { + error_setg(errp, "No block device supports snapshots"); + } return bs; } -- cgit v1.1