Am 20.05.2025 um 12:29 hat Fiona Ebner geschrieben:
> This is in preparation to mark bdrv_drained_begin() as GRAPH_UNLOCKED.
>
> More granular draining is not trivially possible, because
> bdrv_snapshot_delete() can recursively call itself.
>
> The return value of bdrv_all_delete_snapshot() changes from -1 to
> -errno propagated from failed sub-calls. This is fine for the existing
> callers of bdrv_all_delete_snapshot().
>
> Signed-off-by: Fiona Ebner <[email protected]>
> ---
>
> Changes in v2:
> * Use 'must be drained' instead of 'needs to be drained'.
> * Use goto for cleanup/error handling in bdrv_all_delete_snapshot().
> * Don't use atomics to access bs->quiesce_counter.
>
> block/snapshot.c | 26 +++++++++++++++-----------
> blockdev.c | 25 +++++++++++++++++--------
> qemu-img.c | 2 ++
> 3 files changed, 34 insertions(+), 19 deletions(-)
> @@ -571,19 +569,22 @@ int bdrv_all_delete_snapshot(const char *name,
> ERRP_GUARD();
> g_autoptr(GList) bdrvs = NULL;
> GList *iterbdrvs;
> + int ret = 0;
The initialisation here is technically not needed...
> GLOBAL_STATE_CODE();
> - GRAPH_RDLOCK_GUARD_MAINLOOP();
>
> - if (bdrv_all_get_snapshot_devices(has_devices, devices, &bdrvs, errp) <
> 0) {
> - return -1;
> + bdrv_drain_all_begin();
> + bdrv_graph_rdlock_main_loop();
> +
> + ret = bdrv_all_get_snapshot_devices(has_devices, devices, &bdrvs, errp);
> + if (ret < 0) {
> + goto out;
> }
...because ret is assigned here before anyone reads it. But it doesn't
hurt either, so:
Reviewed-by: Kevin Wolf <[email protected]>
