block: Add Error parameter to bdrv_set_backing_hd()

Not all callers of bdrv_set_backing_hd() know for sure that attaching
the backing file will be allowed by the permission system. Return the
error from the function rather than aborting.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Acked-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
This commit is contained in:
Kevin Wolf 2017-02-17 20:42:32 +01:00
parent c8f6d58edb
commit 12fa4af61f
6 changed files with 47 additions and 18 deletions

30
block.c
View File

@ -1883,7 +1883,8 @@ static void bdrv_parent_cb_resize(BlockDriverState *bs)
* Sets the backing file link of a BDS. A new reference is created; callers * Sets the backing file link of a BDS. A new reference is created; callers
* which don't need their own reference any more must call bdrv_unref(). * which don't need their own reference any more must call bdrv_unref().
*/ */
void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd) void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd,
Error **errp)
{ {
if (backing_hd) { if (backing_hd) {
bdrv_ref(backing_hd); bdrv_ref(backing_hd);
@ -1897,9 +1898,12 @@ void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd)
bs->backing = NULL; bs->backing = NULL;
goto out; goto out;
} }
/* FIXME Error handling */
bs->backing = bdrv_attach_child(bs, backing_hd, "backing", &child_backing, bs->backing = bdrv_attach_child(bs, backing_hd, "backing", &child_backing,
&error_abort); errp);
if (!bs->backing) {
bdrv_unref(backing_hd);
}
out: out:
bdrv_refresh_limits(bs, NULL); bdrv_refresh_limits(bs, NULL);
@ -1983,8 +1987,12 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options,
/* Hook up the backing file link; drop our reference, bs owns the /* Hook up the backing file link; drop our reference, bs owns the
* backing_hd reference now */ * backing_hd reference now */
bdrv_set_backing_hd(bs, backing_hd); bdrv_set_backing_hd(bs, backing_hd, &local_err);
bdrv_unref(backing_hd); bdrv_unref(backing_hd);
if (local_err) {
ret = -EINVAL;
goto free_exit;
}
qdict_del(parent_options, bdref_key); qdict_del(parent_options, bdref_key);
@ -2818,7 +2826,7 @@ static void bdrv_close(BlockDriverState *bs)
bs->drv->bdrv_close(bs); bs->drv->bdrv_close(bs);
bs->drv = NULL; bs->drv = NULL;
bdrv_set_backing_hd(bs, NULL); bdrv_set_backing_hd(bs, NULL, &error_abort);
if (bs->file != NULL) { if (bs->file != NULL) {
bdrv_unref_child(bs, bs->file); bdrv_unref_child(bs, bs->file);
@ -2927,7 +2935,8 @@ void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top)
bdrv_ref(bs_top); bdrv_ref(bs_top);
change_parent_backing_link(bs_top, bs_new); change_parent_backing_link(bs_top, bs_new);
bdrv_set_backing_hd(bs_new, bs_top); /* FIXME Error handling */
bdrv_set_backing_hd(bs_new, bs_top, &error_abort);
bdrv_unref(bs_top); bdrv_unref(bs_top);
/* bs_new is now referenced by its new parents, we don't need the /* bs_new is now referenced by its new parents, we don't need the
@ -3075,6 +3084,7 @@ int bdrv_drop_intermediate(BlockDriverState *active, BlockDriverState *top,
BlockDriverState *base, const char *backing_file_str) BlockDriverState *base, const char *backing_file_str)
{ {
BlockDriverState *new_top_bs = NULL; BlockDriverState *new_top_bs = NULL;
Error *local_err = NULL;
int ret = -EIO; int ret = -EIO;
if (!top->drv || !base->drv) { if (!top->drv || !base->drv) {
@ -3107,7 +3117,13 @@ int bdrv_drop_intermediate(BlockDriverState *active, BlockDriverState *top,
if (ret) { if (ret) {
goto exit; goto exit;
} }
bdrv_set_backing_hd(new_top_bs, base);
bdrv_set_backing_hd(new_top_bs, base, &local_err);
if (local_err) {
ret = -EPERM;
error_report_err(local_err);
goto exit;
}
ret = 0; ret = 0;
exit: exit:

View File

@ -121,7 +121,7 @@ static void commit_complete(BlockJob *job, void *opaque)
* filter driver from the backing chain. Do this as the final step so that * filter driver from the backing chain. Do this as the final step so that
* the 'consistent read' permission can be granted. */ * the 'consistent read' permission can be granted. */
if (remove_commit_top_bs) { if (remove_commit_top_bs) {
bdrv_set_backing_hd(overlay_bs, top); bdrv_set_backing_hd(overlay_bs, top, &error_abort);
} }
} }
@ -316,8 +316,8 @@ void commit_start(const char *job_id, BlockDriverState *bs,
goto fail; goto fail;
} }
bdrv_set_backing_hd(commit_top_bs, top); bdrv_set_backing_hd(commit_top_bs, top, &error_abort);
bdrv_set_backing_hd(overlay_bs, commit_top_bs); bdrv_set_backing_hd(overlay_bs, commit_top_bs, &error_abort);
s->commit_top_bs = commit_top_bs; s->commit_top_bs = commit_top_bs;
bdrv_unref(commit_top_bs); bdrv_unref(commit_top_bs);
@ -390,7 +390,7 @@ fail:
blk_unref(s->top); blk_unref(s->top);
} }
if (commit_top_bs) { if (commit_top_bs) {
bdrv_set_backing_hd(overlay_bs, top); bdrv_set_backing_hd(overlay_bs, top, &error_abort);
} }
block_job_unref(&s->common); block_job_unref(&s->common);
} }
@ -451,8 +451,8 @@ int bdrv_commit(BlockDriverState *bs)
goto ro_cleanup; goto ro_cleanup;
} }
bdrv_set_backing_hd(commit_top_bs, backing_file_bs); bdrv_set_backing_hd(commit_top_bs, backing_file_bs, &error_abort);
bdrv_set_backing_hd(bs, commit_top_bs); bdrv_set_backing_hd(bs, commit_top_bs, &error_abort);
ret = blk_insert_bs(backing, backing_file_bs, &local_err); ret = blk_insert_bs(backing, backing_file_bs, &local_err);
if (ret < 0) { if (ret < 0) {
@ -532,7 +532,7 @@ ro_cleanup:
blk_unref(backing); blk_unref(backing);
if (backing_file_bs) { if (backing_file_bs) {
bdrv_set_backing_hd(bs, backing_file_bs); bdrv_set_backing_hd(bs, backing_file_bs, &error_abort);
} }
bdrv_unref(commit_top_bs); bdrv_unref(commit_top_bs);
blk_unref(src); blk_unref(src);

View File

@ -503,6 +503,7 @@ static void mirror_exit(BlockJob *job, void *opaque)
BlockDriverState *src = s->source; BlockDriverState *src = s->source;
BlockDriverState *target_bs = blk_bs(s->target); BlockDriverState *target_bs = blk_bs(s->target);
BlockDriverState *mirror_top_bs = s->mirror_top_bs; BlockDriverState *mirror_top_bs = s->mirror_top_bs;
Error *local_err = NULL;
/* Make sure that the source BDS doesn't go away before we called /* Make sure that the source BDS doesn't go away before we called
* block_job_completed(). */ * block_job_completed(). */
@ -516,7 +517,11 @@ static void mirror_exit(BlockJob *job, void *opaque)
if (s->backing_mode == MIRROR_SOURCE_BACKING_CHAIN) { if (s->backing_mode == MIRROR_SOURCE_BACKING_CHAIN) {
BlockDriverState *backing = s->is_none_mode ? src : s->base; BlockDriverState *backing = s->is_none_mode ? src : s->base;
if (backing_bs(target_bs) != backing) { if (backing_bs(target_bs) != backing) {
bdrv_set_backing_hd(target_bs, backing); bdrv_set_backing_hd(target_bs, backing, &local_err);
if (local_err) {
error_report_err(local_err);
data->ret = -EPERM;
}
} }
} }

View File

@ -68,6 +68,7 @@ static void stream_complete(BlockJob *job, void *opaque)
StreamCompleteData *data = opaque; StreamCompleteData *data = opaque;
BlockDriverState *bs = blk_bs(job->blk); BlockDriverState *bs = blk_bs(job->blk);
BlockDriverState *base = s->base; BlockDriverState *base = s->base;
Error *local_err = NULL;
if (!block_job_is_cancelled(&s->common) && data->reached_end && if (!block_job_is_cancelled(&s->common) && data->reached_end &&
data->ret == 0) { data->ret == 0) {
@ -79,9 +80,15 @@ static void stream_complete(BlockJob *job, void *opaque)
} }
} }
data->ret = bdrv_change_backing_file(bs, base_id, base_fmt); data->ret = bdrv_change_backing_file(bs, base_id, base_fmt);
bdrv_set_backing_hd(bs, base); bdrv_set_backing_hd(bs, base, &local_err);
if (local_err) {
error_report_err(local_err);
data->ret = -EPERM;
goto out;
}
} }
out:
/* Reopen the image back in read-only mode if necessary */ /* Reopen the image back in read-only mode if necessary */
if (s->bs_flags != bdrv_get_flags(bs)) { if (s->bs_flags != bdrv_get_flags(bs)) {
/* Give up write permissions before making it read-only */ /* Give up write permissions before making it read-only */

View File

@ -3041,7 +3041,7 @@ static int enable_write_target(BlockDriverState *bs, Error **errp)
&error_abort); &error_abort);
*(void**) backing->opaque = s; *(void**) backing->opaque = s;
bdrv_set_backing_hd(s->bs, backing); bdrv_set_backing_hd(s->bs, backing, &error_abort);
bdrv_unref(backing); bdrv_unref(backing);
return 0; return 0;

View File

@ -247,7 +247,8 @@ BdrvChild *bdrv_open_child(const char *filename,
BlockDriverState* parent, BlockDriverState* parent,
const BdrvChildRole *child_role, const BdrvChildRole *child_role,
bool allow_none, Error **errp); bool allow_none, Error **errp);
void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd); void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd,
Error **errp);
int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options, int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options,
const char *bdref_key, Error **errp); const char *bdref_key, Error **errp);
BlockDriverState *bdrv_open(const char *filename, const char *reference, BlockDriverState *bdrv_open(const char *filename, const char *reference,