block: Allow changing bs->file on reopen
When the x-blockdev-reopen was added it allowed reconfiguring the
graph by replacing backing files, but changing the 'file' option was
forbidden. Because of this restriction some operations are not
possible, notably inserting and removing block filters.
This patch adds support for replacing the 'file' option. This is
similar to replacing the backing file and the user is likewise
responsible for the correctness of the resulting graph, otherwise this
can lead to data corruption.
Signed-off-by: Alberto Garcia <berto@igalia.com>
[vsementsov: bdrv_reopen_parse_file_or_backing() is modified a lot]
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20210610120537.196183-9-vsementsov@virtuozzo.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
diff --git a/block.c b/block.c
index ca11078..acd35cb 100644
--- a/block.c
+++ b/block.c
@@ -92,7 +92,7 @@
static int bdrv_reopen_prepare(BDRVReopenState *reopen_state,
BlockReopenQueue *queue,
- Transaction *set_backings_tran, Error **errp);
+ Transaction *change_child_tran, Error **errp);
static void bdrv_reopen_commit(BDRVReopenState *reopen_state);
static void bdrv_reopen_abort(BDRVReopenState *reopen_state);
@@ -4148,6 +4148,10 @@
refresh_list = bdrv_topological_dfs(refresh_list, found,
state->old_backing_bs);
}
+ if (state->old_file_bs) {
+ refresh_list = bdrv_topological_dfs(refresh_list, found,
+ state->old_file_bs);
+ }
}
/*
@@ -4240,64 +4244,81 @@
*
* Return 0 on success, otherwise return < 0 and set @errp.
*/
-static int bdrv_reopen_parse_backing(BDRVReopenState *reopen_state,
- Transaction *set_backings_tran,
- Error **errp)
+static int bdrv_reopen_parse_file_or_backing(BDRVReopenState *reopen_state,
+ bool is_backing, Transaction *tran,
+ Error **errp)
{
BlockDriverState *bs = reopen_state->bs;
- BlockDriverState *new_backing_bs;
+ BlockDriverState *new_child_bs;
+ BlockDriverState *old_child_bs = is_backing ? child_bs(bs->backing) :
+ child_bs(bs->file);
+ const char *child_name = is_backing ? "backing" : "file";
QObject *value;
const char *str;
- value = qdict_get(reopen_state->options, "backing");
+ value = qdict_get(reopen_state->options, child_name);
if (value == NULL) {
return 0;
}
switch (qobject_type(value)) {
case QTYPE_QNULL:
- new_backing_bs = NULL;
+ assert(is_backing); /* The 'file' option does not allow a null value */
+ new_child_bs = NULL;
break;
case QTYPE_QSTRING:
str = qstring_get_str(qobject_to(QString, value));
- new_backing_bs = bdrv_lookup_bs(NULL, str, errp);
- if (new_backing_bs == NULL) {
+ new_child_bs = bdrv_lookup_bs(NULL, str, errp);
+ if (new_child_bs == NULL) {
return -EINVAL;
- } else if (bdrv_recurse_has_child(new_backing_bs, bs)) {
- error_setg(errp, "Making '%s' a backing file of '%s' "
- "would create a cycle", str, bs->node_name);
+ } else if (bdrv_recurse_has_child(new_child_bs, bs)) {
+ error_setg(errp, "Making '%s' a %s child of '%s' would create a "
+ "cycle", str, child_name, bs->node_name);
return -EINVAL;
}
break;
default:
- /* 'backing' does not allow any other data type */
+ /*
+ * The options QDict has been flattened, so 'backing' and 'file'
+ * do not allow any other data type here.
+ */
g_assert_not_reached();
}
- if (bs->backing) {
- if (bdrv_skip_implicit_filters(bs->backing->bs) == new_backing_bs) {
+ if (old_child_bs == new_child_bs) {
+ return 0;
+ }
+
+ if (old_child_bs) {
+ if (bdrv_skip_implicit_filters(old_child_bs) == new_child_bs) {
return 0;
}
- if (bs->backing->bs->implicit) {
- error_setg(errp, "Cannot change backing link if '%s' has "
- "an implicit backing file", bs->node_name);
+ if (old_child_bs->implicit) {
+ error_setg(errp, "Cannot replace implicit %s child of %s",
+ child_name, bs->node_name);
return -EPERM;
}
}
- if (bs->drv->is_filter && !bs->backing) {
+ if (bs->drv->is_filter && !old_child_bs) {
/*
* Filters always have a file or a backing child, so we are trying to
* change wrong child
*/
error_setg(errp, "'%s' is a %s filter node that does not support a "
- "backing child", bs->node_name, bs->drv->format_name);
+ "%s child", bs->node_name, bs->drv->format_name, child_name);
return -EINVAL;
}
- reopen_state->old_backing_bs = bs->backing ? bs->backing->bs : NULL;
- return bdrv_set_backing_noperm(bs, new_backing_bs, set_backings_tran, errp);
+ if (is_backing) {
+ reopen_state->old_backing_bs = old_child_bs;
+ } else {
+ reopen_state->old_file_bs = old_child_bs;
+ }
+
+ return bdrv_set_file_or_backing_noperm(bs, new_child_bs, is_backing,
+ tran, errp);
}
/*
@@ -4319,7 +4340,7 @@
*/
static int bdrv_reopen_prepare(BDRVReopenState *reopen_state,
BlockReopenQueue *queue,
- Transaction *set_backings_tran, Error **errp)
+ Transaction *change_child_tran, Error **errp)
{
int ret = -1;
int old_flags;
@@ -4439,12 +4460,21 @@
* either a reference to an existing node (using its node name)
* or NULL to simply detach the current backing file.
*/
- ret = bdrv_reopen_parse_backing(reopen_state, set_backings_tran, errp);
+ ret = bdrv_reopen_parse_file_or_backing(reopen_state, true,
+ change_child_tran, errp);
if (ret < 0) {
goto error;
}
qdict_del(reopen_state->options, "backing");
+ /* Allow changing the 'file' option. In this case NULL is not allowed */
+ ret = bdrv_reopen_parse_file_or_backing(reopen_state, false,
+ change_child_tran, errp);
+ if (ret < 0) {
+ goto error;
+ }
+ qdict_del(reopen_state->options, "file");
+
/* Options that are not handled are only okay if they are unchanged
* compared to the old state. It is expected that some options are only
* used for the initial open, but not reopen (e.g. filename) */
