Am 21.04.2023 um 13:53 hat Vladimir Sementsov-Ogievskiy geschrieben: > We are going to add more block-graph modifying transaction actions, > and block-graph modifying functions are already based on Transaction > API. > > Next, we'll need to separately update permissions after several > graph-modifying actions, and this would be simple with help of > Transaction API. > > So, now let's just transform what we have into new-style transaction > actions. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru> > --- > blockdev.c | 317 +++++++++++++++++++++++++++++++---------------------- > 1 file changed, 186 insertions(+), 131 deletions(-)
> diff --git a/blockdev.c b/blockdev.c > index d7b5c18f0a..293f6a958e 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -1380,10 +1384,9 @@ static void internal_snapshot_abort(BlkActionState > *common) > aio_context_release(aio_context); > } > > -static void internal_snapshot_clean(BlkActionState *common) > +static void internal_snapshot_clean(void *opaque) > { > - InternalSnapshotState *state = DO_UPCAST(InternalSnapshotState, > - common, common); > + InternalSnapshotState *state = opaque; > AioContext *aio_context; > > if (!state->bs) { > @@ -1396,6 +1399,8 @@ static void internal_snapshot_clean(BlkActionState > *common) > bdrv_drained_end(state->bs); > > aio_context_release(aio_context); > + > + g_free(state); > } state is leaked if we take the early return a few lines above: if (!state->bs) { return; } > /* external snapshot private data */ > @@ -1657,6 +1670,8 @@ static void external_snapshot_clean(BlkActionState > *common) > bdrv_unref(state->new_bs); > > aio_context_release(aio_context); > + > + g_free(state); > } Same potential leak of state. > typedef struct DriveBackupState { > @@ -1856,6 +1883,8 @@ static void drive_backup_clean(BlkActionState *common) > bdrv_drained_end(state->bs); > > aio_context_release(aio_context); > + > + g_free(state); > } Here as well. > typedef struct BlockdevBackupState { > @@ -1950,6 +1991,8 @@ static void blockdev_backup_clean(BlkActionState > *common) > bdrv_drained_end(state->bs); > > aio_context_release(aio_context); > + > + g_free(state); > } And here. Other than that, the patch looks good to me. Kevin