Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
blockdev: transaction: refactor handling transaction properties 
Only backup supports GROUPED mode. Make this logic more clear. And
avoid passing extra thing to each action.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Message-Id: <20230510150624.310640-5-vsementsov@yandex-team.ru>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
  • Loading branch information
Vladimir Sementsov-Ogievskiy authored and Kevin Wolf committed May 19, 2023
1 parent 30c96b5 commit c85f34c
Showing 1 changed file with 22 additions and 74 deletions.
96 changes: 22 additions & 74 deletions blockdev.c
View file
Expand Up @@ -1230,7 +1230,6 @@ struct BlkActionState {
TransactionAction *action;
const BlkActionOps *ops;
JobTxn *block_job_txn;
TransactionProperties *txn_props;
QTAILQ_ENTRY(BlkActionState) entry;
};

Expand All @@ -1249,19 +1248,6 @@ TransactionActionDrv internal_snapshot_drv = {
.clean = internal_snapshot_clean,
};

static int action_check_completion_mode(BlkActionState *s, Error **errp)
{
if (s->txn_props->completion_mode != ACTION_COMPLETION_MODE_INDIVIDUAL) {
error_setg(errp,
"Action '%s' does not support Transaction property "
"completion-mode = %s",
TransactionActionKind_str(s->action->type),
ActionCompletionMode_str(s->txn_props->completion_mode));
return -1;
}
return 0;
}

static void internal_snapshot_action(BlkActionState *common,
Transaction *tran, Error **errp)
{
Expand All @@ -1284,15 +1270,9 @@ static void internal_snapshot_action(BlkActionState *common,

tran_add(tran, &internal_snapshot_drv, state);

/* 1. parse input */
device = internal->device;
name = internal->name;

/* 2. check for validation */
if (action_check_completion_mode(common, errp) < 0) {
return;
}

bs = qmp_get_root_bs(device, errp);
if (!bs) {
return;
Expand Down Expand Up @@ -1476,9 +1456,6 @@ static void external_snapshot_action(BlkActionState *common, Transaction *tran,
}

/* start processing */
if (action_check_completion_mode(common, errp) < 0) {
return;
}

state->old_bs = bdrv_lookup_bs(device, node_name, errp);
if (!state->old_bs) {
Expand Down Expand Up @@ -2022,10 +1999,6 @@ static void block_dirty_bitmap_add_action(BlkActionState *common,

tran_add(tran, &block_dirty_bitmap_add_drv, state);

if (action_check_completion_mode(common, errp) < 0) {
return;
}

action = common->action->u.block_dirty_bitmap_add.data;
/* AIO context taken and released within qmp_block_dirty_bitmap_add */
qmp_block_dirty_bitmap_add(action->node, action->name,
Expand Down Expand Up @@ -2072,10 +2045,6 @@ static void block_dirty_bitmap_clear_action(BlkActionState *common,

tran_add(tran, &block_dirty_bitmap_clear_drv, state);

if (action_check_completion_mode(common, errp) < 0) {
return;
}

action = common->action->u.block_dirty_bitmap_clear.data;
state->bitmap = block_dirty_bitmap_lookup(action->node,
action->name,
Expand Down Expand Up @@ -2123,10 +2092,6 @@ static void block_dirty_bitmap_enable_action(BlkActionState *common,

tran_add(tran, &block_dirty_bitmap_enable_drv, state);

if (action_check_completion_mode(common, errp) < 0) {
return;
}

action = common->action->u.block_dirty_bitmap_enable.data;
state->bitmap = block_dirty_bitmap_lookup(action->node,
action->name,
Expand Down Expand Up @@ -2168,10 +2133,6 @@ static void block_dirty_bitmap_disable_action(BlkActionState *common,

tran_add(tran, &block_dirty_bitmap_disable_drv, state);

if (action_check_completion_mode(common, errp) < 0) {
return;
}

action = common->action->u.block_dirty_bitmap_disable.data;
state->bitmap = block_dirty_bitmap_lookup(action->node,
action->name,
Expand Down Expand Up @@ -2213,10 +2174,6 @@ static void block_dirty_bitmap_merge_action(BlkActionState *common,

tran_add(tran, &block_dirty_bitmap_merge_drv, state);

if (action_check_completion_mode(common, errp) < 0) {
return;
}

action = common->action->u.block_dirty_bitmap_merge.data;

state->bitmap = block_dirty_bitmap_merge(action->node, action->target,
Expand All @@ -2241,10 +2198,6 @@ static void block_dirty_bitmap_remove_action(BlkActionState *common,

tran_add(tran, &block_dirty_bitmap_remove_drv, state);

if (action_check_completion_mode(common, errp) < 0) {
return;
}

action = common->action->u.block_dirty_bitmap_remove.data;

state->bitmap = block_dirty_bitmap_remove(action->node, action->name,
Expand Down Expand Up @@ -2348,25 +2301,6 @@ static const BlkActionOps actions_map[] = {
*/
};

/**
* Allocate a TransactionProperties structure if necessary, and fill
* that structure with desired defaults if they are unset.
*/
static TransactionProperties *get_transaction_properties(
TransactionProperties *props)
{
if (!props) {
props = g_new0(TransactionProperties, 1);
}

if (!props->has_completion_mode) {
props->has_completion_mode = true;
props->completion_mode = ACTION_COMPLETION_MODE_INDIVIDUAL;
}

return props;
}

/*
* 'Atomic' group operations. The operations are performed as a set, and if
* any fail then we roll back all operations in the group.
Expand All @@ -2378,24 +2312,42 @@ void qmp_transaction(TransactionActionList *actions,
Error **errp)
{
TransactionActionList *act;
bool has_properties = !!properties;
JobTxn *block_job_txn = NULL;
Error *local_err = NULL;
Transaction *tran = tran_new();
Transaction *tran;
ActionCompletionMode comp_mode =
properties ? properties->completion_mode :
ACTION_COMPLETION_MODE_INDIVIDUAL;

GLOBAL_STATE_CODE();

/* Does this transaction get canceled as a group on failure?
* If not, we don't really need to make a JobTxn.
*/
properties = get_transaction_properties(properties);
if (properties->completion_mode != ACTION_COMPLETION_MODE_INDIVIDUAL) {
if (comp_mode != ACTION_COMPLETION_MODE_INDIVIDUAL) {
for (act = actions; act; act = act->next) {
TransactionActionKind type = act->value->type;

if (type != TRANSACTION_ACTION_KIND_BLOCKDEV_BACKUP &&
type != TRANSACTION_ACTION_KIND_DRIVE_BACKUP)
{
error_setg(errp,
"Action '%s' does not support transaction property "
"completion-mode = %s",
TransactionActionKind_str(type),
ActionCompletionMode_str(comp_mode));
return;
}
}

block_job_txn = job_txn_new();
}

/* drain all i/o before any operations */
bdrv_drain_all();

tran = tran_new();

/* We don't do anything in this loop that commits us to the operations */
for (act = actions; act; act = act->next) {
TransactionAction *dev_info = act->value;
Expand All @@ -2411,7 +2363,6 @@ void qmp_transaction(TransactionActionList *actions,
state->ops = ops;
state->action = dev_info;
state->block_job_txn = block_job_txn;
state->txn_props = properties;

state->ops->action(state, tran, &local_err);
if (local_err) {
Expand All @@ -2429,9 +2380,6 @@ void qmp_transaction(TransactionActionList *actions,
/* failure, and it is all-or-none; roll back all operations */
tran_abort(tran);
exit:
if (!has_properties) {
qapi_free_TransactionProperties(properties);
}
job_txn_unref(block_job_txn);
}

Expand Down

0 comments on commit c85f34c

Please sign in to comment.