block: push error reporting into bdrv_all_*_snapshot functions

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 <eblake@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
[PMD: Initialize variables]
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20210204124834.774401-2-berrange@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
This commit is contained in:
Daniel P. Berrangé 2021-02-04 12:48:23 +00:00 committed by Dr. David Alan Gilbert
parent a64aec725e
commit e26f98e209
7 changed files with 68 additions and 90 deletions

View File

@ -900,10 +900,11 @@ void hmp_info_snapshots(Monitor *mon, const QDict *qdict)
ImageEntry *image_entry, *next_ie; ImageEntry *image_entry, *next_ie;
SnapshotEntry *snapshot_entry; SnapshotEntry *snapshot_entry;
Error *err = NULL;
bs = bdrv_all_find_vmstate_bs(); bs = bdrv_all_find_vmstate_bs(&err);
if (!bs) { if (!bs) {
monitor_printf(mon, "No available block device supports snapshots\n"); error_report_err(err);
return; return;
} }
aio_context = bdrv_get_aio_context(bs); aio_context = bdrv_get_aio_context(bs);
@ -953,7 +954,7 @@ void hmp_info_snapshots(Monitor *mon, const QDict *qdict)
total = 0; total = 0;
for (i = 0; i < nb_sns; i++) { for (i = 0; i < nb_sns; i++) {
SnapshotEntry *next_sn; 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; global_snapshots[total] = i;
total++; total++;
QTAILQ_FOREACH(image_entry, &image_list, next) { QTAILQ_FOREACH(image_entry, &image_list, next) {

View File

@ -462,14 +462,14 @@ static bool bdrv_all_snapshots_includes_bs(BlockDriverState *bs)
* These functions will properly handle dataplane (take aio_context_acquire * These functions will properly handle dataplane (take aio_context_acquire
* when appropriate for appropriate block drivers) */ * 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; BlockDriverState *bs;
BdrvNextIterator it; BdrvNextIterator it;
for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) { for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
AioContext *ctx = bdrv_get_aio_context(bs); AioContext *ctx = bdrv_get_aio_context(bs);
bool ok = true;
aio_context_acquire(ctx); aio_context_acquire(ctx);
if (bdrv_all_snapshots_includes_bs(bs)) { if (bdrv_all_snapshots_includes_bs(bs)) {
@ -477,26 +477,25 @@ bool bdrv_all_can_snapshot(BlockDriverState **first_bad_bs)
} }
aio_context_release(ctx); aio_context_release(ctx);
if (!ok) { 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); bdrv_next_cleanup(&it);
goto fail; return false;
} }
} }
fail: return true;
*first_bad_bs = bs;
return ok;
} }
int bdrv_all_delete_snapshot(const char *name, BlockDriverState **first_bad_bs, int bdrv_all_delete_snapshot(const char *name, Error **errp)
Error **errp)
{ {
int ret = 0;
BlockDriverState *bs; BlockDriverState *bs;
BdrvNextIterator it; BdrvNextIterator it;
QEMUSnapshotInfo sn1, *snapshot = &sn1; QEMUSnapshotInfo sn1, *snapshot = &sn1;
for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) { for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
AioContext *ctx = bdrv_get_aio_context(bs); AioContext *ctx = bdrv_get_aio_context(bs);
int ret = 0;
aio_context_acquire(ctx); aio_context_acquire(ctx);
if (bdrv_all_snapshots_includes_bs(bs) && 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); aio_context_release(ctx);
if (ret < 0) { 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); bdrv_next_cleanup(&it);
goto fail; return -1;
} }
} }
fail: return 0;
*first_bad_bs = bs;
return ret;
} }
int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs, int bdrv_all_goto_snapshot(const char *name, Error **errp)
Error **errp)
{ {
int ret = 0;
BlockDriverState *bs; BlockDriverState *bs;
BdrvNextIterator it; BdrvNextIterator it;
for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) { for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
AioContext *ctx = bdrv_get_aio_context(bs); AioContext *ctx = bdrv_get_aio_context(bs);
int ret = 0;
aio_context_acquire(ctx); aio_context_acquire(ctx);
if (bdrv_all_snapshots_includes_bs(bs)) { 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); aio_context_release(ctx);
if (ret < 0) { 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); bdrv_next_cleanup(&it);
goto fail; return -1;
} }
} }
fail: return 0;
*first_bad_bs = bs;
return ret;
} }
int bdrv_all_find_snapshot(const char *name, BlockDriverState **first_bad_bs) int bdrv_all_find_snapshot(const char *name, Error **errp)
{ {
QEMUSnapshotInfo sn; QEMUSnapshotInfo sn;
int err = 0;
BlockDriverState *bs; BlockDriverState *bs;
BdrvNextIterator it; BdrvNextIterator it;
for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) { for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
AioContext *ctx = bdrv_get_aio_context(bs); AioContext *ctx = bdrv_get_aio_context(bs);
int ret = 0;
aio_context_acquire(ctx); aio_context_acquire(ctx);
if (bdrv_all_snapshots_includes_bs(bs)) { 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); 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); bdrv_next_cleanup(&it);
goto fail; return -1;
} }
} }
fail: return 0;
*first_bad_bs = bs;
return err;
} }
int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn, int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn,
BlockDriverState *vm_state_bs, BlockDriverState *vm_state_bs,
uint64_t vm_state_size, uint64_t vm_state_size,
BlockDriverState **first_bad_bs) Error **errp)
{ {
int err = 0;
BlockDriverState *bs; BlockDriverState *bs;
BdrvNextIterator it; BdrvNextIterator it;
for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) { for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
AioContext *ctx = bdrv_get_aio_context(bs); AioContext *ctx = bdrv_get_aio_context(bs);
int ret = 0;
aio_context_acquire(ctx); aio_context_acquire(ctx);
if (bs == vm_state_bs) { if (bs == vm_state_bs) {
sn->vm_state_size = vm_state_size; 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)) { } else if (bdrv_all_snapshots_includes_bs(bs)) {
sn->vm_state_size = 0; sn->vm_state_size = 0;
err = bdrv_snapshot_create(bs, sn); ret = bdrv_snapshot_create(bs, sn);
} }
aio_context_release(ctx); 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); bdrv_next_cleanup(&it);
goto fail; return -1;
} }
} }
fail: return 0;
*first_bad_bs = bs;
return err;
} }
BlockDriverState *bdrv_all_find_vmstate_bs(void) BlockDriverState *bdrv_all_find_vmstate_bs(Error **errp)
{ {
BlockDriverState *bs; BlockDriverState *bs;
BdrvNextIterator it; BdrvNextIterator it;
@ -620,5 +618,8 @@ BlockDriverState *bdrv_all_find_vmstate_bs(void)
break; break;
} }
} }
if (!bs) {
error_setg(errp, "No block device supports snapshots");
}
return bs; return bs;
} }

View File

@ -77,17 +77,15 @@ int bdrv_snapshot_load_tmp_by_id_or_name(BlockDriverState *bs,
* These functions will properly handle dataplane (take aio_context_acquire * These functions will properly handle dataplane (take aio_context_acquire
* when appropriate for appropriate block drivers */ * when appropriate for appropriate block drivers */
bool bdrv_all_can_snapshot(BlockDriverState **first_bad_bs); bool bdrv_all_can_snapshot(Error **errp);
int bdrv_all_delete_snapshot(const char *name, BlockDriverState **first_bsd_bs, int bdrv_all_delete_snapshot(const char *name, Error **errp);
Error **errp); int bdrv_all_goto_snapshot(const char *name, Error **errp);
int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs, int bdrv_all_find_snapshot(const char *name, Error **errp);
Error **errp);
int bdrv_all_find_snapshot(const char *name, BlockDriverState **first_bad_bs);
int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn, int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn,
BlockDriverState *vm_state_bs, BlockDriverState *vm_state_bs,
uint64_t vm_state_size, uint64_t vm_state_size,
BlockDriverState **first_bad_bs); Error **errp);
BlockDriverState *bdrv_all_find_vmstate_bs(void); BlockDriverState *bdrv_all_find_vmstate_bs(Error **errp);
#endif #endif

View File

@ -2766,7 +2766,7 @@ int qemu_load_device_state(QEMUFile *f)
int save_snapshot(const char *name, Error **errp) int save_snapshot(const char *name, Error **errp)
{ {
BlockDriverState *bs, *bs1; BlockDriverState *bs;
QEMUSnapshotInfo sn1, *sn = &sn1; QEMUSnapshotInfo sn1, *sn = &sn1;
int ret = -1, ret2; int ret = -1, ret2;
QEMUFile *f; QEMUFile *f;
@ -2786,25 +2786,19 @@ int save_snapshot(const char *name, Error **errp)
return ret; return ret;
} }
if (!bdrv_all_can_snapshot(&bs)) { if (!bdrv_all_can_snapshot(errp)) {
error_setg(errp, "Device '%s' is writable but does not support "
"snapshots", bdrv_get_device_or_node_name(bs));
return ret; return ret;
} }
/* Delete old snapshots of the same name */ /* Delete old snapshots of the same name */
if (name) { if (name) {
ret = bdrv_all_delete_snapshot(name, &bs1, errp); if (bdrv_all_delete_snapshot(name, errp) < 0) {
if (ret < 0) {
error_prepend(errp, "Error while deleting snapshot on device "
"'%s': ", bdrv_get_device_or_node_name(bs1));
return ret; return ret;
} }
} }
bs = bdrv_all_find_vmstate_bs(); bs = bdrv_all_find_vmstate_bs(errp);
if (bs == NULL) { if (bs == NULL) {
error_setg(errp, "No block device can accept snapshots");
return ret; return ret;
} }
aio_context = bdrv_get_aio_context(bs); aio_context = bdrv_get_aio_context(bs);
@ -2868,11 +2862,9 @@ int save_snapshot(const char *name, Error **errp)
aio_context_release(aio_context); aio_context_release(aio_context);
aio_context = NULL; aio_context = NULL;
ret = bdrv_all_create_snapshot(sn, bs, vm_state_size, &bs); ret = bdrv_all_create_snapshot(sn, bs, vm_state_size, errp);
if (ret < 0) { if (ret < 0) {
error_setg(errp, "Error while creating snapshot on '%s'", bdrv_all_delete_snapshot(sn->name, NULL);
bdrv_get_device_or_node_name(bs));
bdrv_all_delete_snapshot(sn->name, &bs, NULL);
goto the_end; goto the_end;
} }
@ -2975,30 +2967,23 @@ void qmp_xen_load_devices_state(const char *filename, Error **errp)
int load_snapshot(const char *name, Error **errp) int load_snapshot(const char *name, Error **errp)
{ {
BlockDriverState *bs, *bs_vm_state; BlockDriverState *bs_vm_state;
QEMUSnapshotInfo sn; QEMUSnapshotInfo sn;
QEMUFile *f; QEMUFile *f;
int ret; int ret;
AioContext *aio_context; AioContext *aio_context;
MigrationIncomingState *mis = migration_incoming_get_current(); MigrationIncomingState *mis = migration_incoming_get_current();
if (!bdrv_all_can_snapshot(&bs)) { if (!bdrv_all_can_snapshot(errp)) {
error_setg(errp,
"Device '%s' is writable but does not support snapshots",
bdrv_get_device_or_node_name(bs));
return -ENOTSUP; return -ENOTSUP;
} }
ret = bdrv_all_find_snapshot(name, &bs); ret = bdrv_all_find_snapshot(name, errp);
if (ret < 0) { if (ret < 0) {
error_setg(errp,
"Device '%s' does not have the requested snapshot '%s'",
bdrv_get_device_or_node_name(bs), name);
return ret; return ret;
} }
bs_vm_state = bdrv_all_find_vmstate_bs(); bs_vm_state = bdrv_all_find_vmstate_bs(errp);
if (!bs_vm_state) { if (!bs_vm_state) {
error_setg(errp, "No block device supports snapshots");
return -ENOTSUP; return -ENOTSUP;
} }
aio_context = bdrv_get_aio_context(bs_vm_state); aio_context = bdrv_get_aio_context(bs_vm_state);
@ -3024,10 +3009,8 @@ int load_snapshot(const char *name, Error **errp)
/* Flush all IO requests so they don't interfere with the new state. */ /* Flush all IO requests so they don't interfere with the new state. */
bdrv_drain_all_begin(); bdrv_drain_all_begin();
ret = bdrv_all_goto_snapshot(name, &bs, errp); ret = bdrv_all_goto_snapshot(name, errp);
if (ret < 0) { if (ret < 0) {
error_prepend(errp, "Could not load snapshot '%s' on '%s': ",
name, bdrv_get_device_or_node_name(bs));
goto err_drain; goto err_drain;
} }

View File

@ -1155,15 +1155,10 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
void hmp_delvm(Monitor *mon, const QDict *qdict) void hmp_delvm(Monitor *mon, const QDict *qdict)
{ {
BlockDriverState *bs;
Error *err = NULL; Error *err = NULL;
const char *name = qdict_get_str(qdict, "name"); const char *name = qdict_get_str(qdict, "name");
if (bdrv_all_delete_snapshot(name, &bs, &err) < 0) { bdrv_all_delete_snapshot(name, &err);
error_prepend(&err,
"deleting snapshot on device '%s': ",
bdrv_get_device_name(bs));
}
hmp_handle_error(mon, err); hmp_handle_error(mon, err);
} }

View File

@ -148,7 +148,7 @@ static char *replay_find_nearest_snapshot(int64_t icount,
*snapshot_icount = -1; *snapshot_icount = -1;
bs = bdrv_all_find_vmstate_bs(); bs = bdrv_all_find_vmstate_bs(NULL);
if (!bs) { if (!bs) {
goto fail; goto fail;
} }
@ -159,7 +159,7 @@ static char *replay_find_nearest_snapshot(int64_t icount,
aio_context_release(aio_context); aio_context_release(aio_context);
for (i = 0; i < nb_sns; i++) { for (i = 0; i < nb_sns; i++) {
if (bdrv_all_find_snapshot(sn_tab[i].name, &bs) == 0) { if (bdrv_all_find_snapshot(sn_tab[i].name, NULL) == 0) {
if (sn_tab[i].icount != -1ULL if (sn_tab[i].icount != -1ULL
&& sn_tab[i].icount <= icount && sn_tab[i].icount <= icount
&& (!nearest || nearest->icount < sn_tab[i].icount)) { && (!nearest || nearest->icount < sn_tab[i].icount)) {

View File

@ -6,9 +6,9 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
Testing: Testing:
QEMU X.Y.Z monitor - type 'help' for more information QEMU X.Y.Z monitor - type 'help' for more information
(qemu) savevm snap0 (qemu) savevm snap0
Error: No block device can accept snapshots Error: No block device supports snapshots
(qemu) info snapshots (qemu) info snapshots
No available block device supports snapshots No block device supports snapshots
(qemu) loadvm snap0 (qemu) loadvm snap0
Error: No block device supports snapshots Error: No block device supports snapshots
(qemu) quit (qemu) quit
@ -22,7 +22,7 @@ QEMU X.Y.Z monitor - type 'help' for more information
(qemu) savevm snap0 (qemu) savevm snap0
Error: Device 'none0' is writable but does not support snapshots Error: Device 'none0' is writable but does not support snapshots
(qemu) info snapshots (qemu) info snapshots
No available block device supports snapshots No block device supports snapshots
(qemu) loadvm snap0 (qemu) loadvm snap0
Error: Device 'none0' is writable but does not support snapshots Error: Device 'none0' is writable but does not support snapshots
(qemu) quit (qemu) quit
@ -58,7 +58,7 @@ QEMU X.Y.Z monitor - type 'help' for more information
(qemu) savevm snap0 (qemu) savevm snap0
Error: Device 'virtio0' is writable but does not support snapshots Error: Device 'virtio0' is writable but does not support snapshots
(qemu) info snapshots (qemu) info snapshots
No available block device supports snapshots No block device supports snapshots
(qemu) loadvm snap0 (qemu) loadvm snap0
Error: Device 'virtio0' is writable but does not support snapshots Error: Device 'virtio0' is writable but does not support snapshots
(qemu) quit (qemu) quit
@ -83,7 +83,7 @@ QEMU X.Y.Z monitor - type 'help' for more information
(qemu) savevm snap0 (qemu) savevm snap0
Error: Device 'file' is writable but does not support snapshots Error: Device 'file' is writable but does not support snapshots
(qemu) info snapshots (qemu) info snapshots
No available block device supports snapshots No block device supports snapshots
(qemu) loadvm snap0 (qemu) loadvm snap0
Error: Device 'file' is writable but does not support snapshots Error: Device 'file' is writable but does not support snapshots
(qemu) quit (qemu) quit