Am 19.05.2014 um 16:10 hat Markus Armbruster geschrieben:
> Fam Zheng <f...@redhat.com> writes:
>
> > BlockDriverState.op_blockers is an array of lists with BLOCK_OP_TYPE_MAX
> > elements. Each list is a list of blockers of an operation type
> > (BlockOpType), that marks this BDS as currently blocked for a certain
> > type of operation with reason errors stored in the list. The rule of
> > usage is:
> >
> > * BDS user who wants to take an operation should check if there's any
> > blocker of the type with bdrv_op_is_blocked().
> >
> > * BDS user who wants to block certain types of operation, should call
> > bdrv_op_block (or bdrv_op_block_all to block all types of operations,
> > which is similar to the existing bdrv_set_in_use()).
> >
> > * A blocker is only referenced by op_blockers, so the lifecycle is
> > managed by caller, and shouldn't be lost until unblock, so typically
> > a caller does these:
> >
> > - Allocate a blocker with error_setg or similar, call bdrv_op_block()
> > to block some operations.
> > - Hold the blocker, do his job.
> > - Unblock operations that it blocked, with the same reason pointer
> > passed to bdrv_op_unblock().
> > - Release the blocker with error_free().
> >
> > Signed-off-by: Fam Zheng <f...@redhat.com>
> > Reviewed-by: Benoit Canet <ben...@irqsave.net>
> > Reviewed-by: Jeff Cody <jc...@redhat.com>
> > ---
> > block.c | 75
> > +++++++++++++++++++++++++++++++++++++++++++++++
> > include/block/block.h | 7 +++++
> > include/block/block_int.h | 5 ++++
> > 3 files changed, 87 insertions(+)
> >
> > diff --git a/block.c b/block.c
> > index b749d31..32338ca 100644
> > --- a/block.c
> > +++ b/block.c
> > @@ -335,6 +335,7 @@ void bdrv_register(BlockDriver *bdrv)
> > BlockDriverState *bdrv_new(const char *device_name, Error **errp)
> > {
> > BlockDriverState *bs;
> > + int i;
> >
> > if (bdrv_find(device_name)) {
> > error_setg(errp, "Device with id '%s' already exists",
> > @@ -353,6 +354,9 @@ BlockDriverState *bdrv_new(const char *device_name,
> > Error **errp)
> > if (device_name[0] != '\0') {
> > QTAILQ_INSERT_TAIL(&bdrv_states, bs, device_list);
> > }
> > + for (i = 0; i < BLOCK_OP_TYPE_MAX; i++) {
> > + QLIST_INIT(&bs->op_blockers[i]);
> > + }
> > bdrv_iostatus_disable(bs);
> > notifier_list_init(&bs->close_notifiers);
> > notifier_with_return_list_init(&bs->before_write_notifiers);
> > @@ -1907,6 +1911,8 @@ static void bdrv_move_feature_fields(BlockDriverState
> > *bs_dest,
> > pstrcpy(bs_dest->device_name, sizeof(bs_dest->device_name),
> > bs_src->device_name);
> > bs_dest->device_list = bs_src->device_list;
> > + memcpy(bs_dest->op_blockers, bs_src->op_blockers,
> > + sizeof(bs_dest->op_blockers));
> > }
> >
> > /*
> > @@ -5269,6 +5275,75 @@ void bdrv_unref(BlockDriverState *bs)
> > }
> > }
> >
> > +struct BdrvOpBlocker {
> > + Error *reason;
> > + QLIST_ENTRY(BdrvOpBlocker) list;
> > +};
> > +
> > +bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType op, Error **errp)
> > +{
> > + BdrvOpBlocker *blocker;
> > + assert((int) op >= 0 && op < BLOCK_OP_TYPE_MAX);
>
> Space between cast and its operand is unusual. Please don't respin just
> for that.
That was a surprising statement for me. Do you have an idea how to grep
for casts? I tried '*)' just in order to find _some_ examples of casts,
and there doesn't seem to be a clear winner. But if there is one, it
appears to be the version with space.
(I won't reject patches with either style.)
Kevin
