Message ID | 1445495580-32129-10-git-send-email-famz@redhat.com |
---|---|
State | New |
Headers | show |
Am 22.10.2015 um 08:32 hat Fam Zheng geschrieben: > This ensures the atomicity of the transaction by avoiding processing of > external requests such as those from ioeventfd. > > state->bs is assigned right after bdrv_drained_begin. Because it was > used as the flag for deletion or not in abort, now we need a separate > flag - InternalSnapshotState.created. > > Signed-off-by: Fam Zheng <famz@redhat.com> > --- > blockdev.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/blockdev.c b/blockdev.c > index 52f44b2..adc0e69 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -1370,6 +1370,7 @@ typedef struct InternalSnapshotState { > BlockDriverState *bs; > AioContext *aio_context; > QEMUSnapshotInfo sn; > + bool created; > } InternalSnapshotState; > > static void internal_snapshot_prepare(BlkTransactionState *common, > @@ -1414,6 +1415,9 @@ static void internal_snapshot_prepare(BlkTransactionState *common, > } > bs = blk_bs(blk); > > + state->bs = bs; > + bdrv_drained_begin(bs); > + > if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_INTERNAL_SNAPSHOT, errp)) { > return; > } More context: /* AioContext is released in .clean() */ state->aio_context = blk_get_aio_context(blk); aio_context_acquire(state->aio_context); if (!blk_is_available(blk)) { error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, device); return; } bs = blk_bs(blk); state->bs = bs; bdrv_drained_begin(bs); If we error out because of !blk_is_available(blk), we will still call bdrv_drained_end() in .clean even though bdrv_drained_begin() hasn't been called yet. Kevin
On Thu, Oct 22, 2015 at 12:18:11PM +0200, Kevin Wolf wrote: > Am 22.10.2015 um 08:32 hat Fam Zheng geschrieben: > > This ensures the atomicity of the transaction by avoiding processing of > > external requests such as those from ioeventfd. > > > > state->bs is assigned right after bdrv_drained_begin. Because it was > > used as the flag for deletion or not in abort, now we need a separate > > flag - InternalSnapshotState.created. > > > > Signed-off-by: Fam Zheng <famz@redhat.com> > > --- > > blockdev.c | 9 +++++++-- > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/blockdev.c b/blockdev.c > > index 52f44b2..adc0e69 100644 > > --- a/blockdev.c > > +++ b/blockdev.c > > @@ -1370,6 +1370,7 @@ typedef struct InternalSnapshotState { > > BlockDriverState *bs; > > AioContext *aio_context; > > QEMUSnapshotInfo sn; > > + bool created; > > } InternalSnapshotState; > > > > static void internal_snapshot_prepare(BlkTransactionState *common, > > @@ -1414,6 +1415,9 @@ static void internal_snapshot_prepare(BlkTransactionState *common, > > } > > bs = blk_bs(blk); > > > > + state->bs = bs; > > + bdrv_drained_begin(bs); > > + > > if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_INTERNAL_SNAPSHOT, errp)) { > > return; > > } > > More context: > > /* AioContext is released in .clean() */ > state->aio_context = blk_get_aio_context(blk); > aio_context_acquire(state->aio_context); > > if (!blk_is_available(blk)) { > error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, device); > return; > } > bs = blk_bs(blk); > > state->bs = bs; > bdrv_drained_begin(bs); > > If we error out because of !blk_is_available(blk), we will still call > bdrv_drained_end() in .clean even though bdrv_drained_begin() hasn't > been called yet. > Right, if the bdrv_drained_begin() is moved down as it is in this patch, then the state->aio_context assignment needs moved down as well.
diff --git a/blockdev.c b/blockdev.c index 52f44b2..adc0e69 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1370,6 +1370,7 @@ typedef struct InternalSnapshotState { BlockDriverState *bs; AioContext *aio_context; QEMUSnapshotInfo sn; + bool created; } InternalSnapshotState; static void internal_snapshot_prepare(BlkTransactionState *common, @@ -1414,6 +1415,9 @@ static void internal_snapshot_prepare(BlkTransactionState *common, } bs = blk_bs(blk); + state->bs = bs; + bdrv_drained_begin(bs); + if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_INTERNAL_SNAPSHOT, errp)) { return; } @@ -1465,7 +1469,7 @@ static void internal_snapshot_prepare(BlkTransactionState *common, } /* 4. succeed, mark a snapshot is created */ - state->bs = bs; + state->created = true; } static void internal_snapshot_abort(BlkTransactionState *common) @@ -1476,7 +1480,7 @@ static void internal_snapshot_abort(BlkTransactionState *common) QEMUSnapshotInfo *sn = &state->sn; Error *local_error = NULL; - if (!bs) { + if (!state->created) { return; } @@ -1497,6 +1501,7 @@ static void internal_snapshot_clean(BlkTransactionState *common) common, common); if (state->aio_context) { + bdrv_drained_end(state->bs); aio_context_release(state->aio_context); } }
This ensures the atomicity of the transaction by avoiding processing of external requests such as those from ioeventfd. state->bs is assigned right after bdrv_drained_begin. Because it was used as the flag for deletion or not in abort, now we need a separate flag - InternalSnapshotState.created. Signed-off-by: Fam Zheng <famz@redhat.com> --- blockdev.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)