Am 22.11.2013 um 06:24 hat Fam Zheng geschrieben:
> Signed-off-by: Fam Zheng <f...@redhat.com>
> ---
> block.c | 37 ++++++++++++++++++++++++++++++++-----
> include/block/block_int.h | 3 +++
> 2 files changed, 35 insertions(+), 5 deletions(-)
>
> diff --git a/block.c b/block.c
> index 3bf4c8a..a5da656 100644
> --- a/block.c
> +++ b/block.c
> @@ -1166,11 +1166,33 @@ int bdrv_open(BlockDriverState *bs, const char
> *filename, QDict *options,
> /* If there is a backing file, use it */
> if ((flags & BDRV_O_NO_BACKING) == 0) {
> QDict *backing_options;
> -
> - qdict_extract_subqdict(options, &backing_options, "backing.");
> - ret = bdrv_open_backing_file(bs, backing_options, &local_err);
> - if (ret < 0) {
> - goto close_and_fail;
> + const char *backing_bs;
> +
> + backing_bs = qdict_get_try_str(options, "backing");
> + if (backing_bs) {
> + bs->backing_hd = bdrv_find(backing_bs);
> + qdict_del(options, "backing");
> + if (bs->backing_hd) {
> + bdrv_ref(bs->backing_hd);
> + assert(!bs->backing_blocker);
> + error_setg(&bs->backing_blocker,
> + "device is used as backing hd of '%s'",
> + bs->device_name);
> + bdrv_op_block_all(bs->backing_hd, bs->backing_blocker);
> + pstrcpy(bs->backing_file, sizeof(bs->backing_file),
> + bs->backing_hd->filename);
> + pstrcpy(bs->backing_format, sizeof(bs->backing_format),
> + bs->backing_hd->drv->format_name);
> + } else {
> + error_setg(errp, "Backing device not found: %s", backing_bs);
> + goto close_and_fail;
> + }
> + } else {
> + qdict_extract_subqdict(options, &backing_options, "backing.");
> + ret = bdrv_open_backing_file(bs, backing_options, &local_err);
> + if (ret < 0) {
> + goto close_and_fail;
> + }
> }
> }
Can we move all of this code into bdrv_open_backing_file()?
I think the bdrv_op_block_all() part should apply to all kinds of
backing files. Currently you can't issue any monitor commands, so it
doesn't matter yet, but Benoît is working on adding per-node names and
then we might want this.
And consistency never hurts anyway.
> @@ -1461,6 +1483,11 @@ void bdrv_close(BlockDriverState *bs)
>
> if (bs->drv) {
> if (bs->backing_hd) {
> + if (error_is_set(&bs->backing_blocker)) {
Then this one should become unconditional as well, I think.
> + bdrv_op_unblock_all(bs->backing_hd,
> + bs->backing_blocker);
> + error_free(bs->backing_blocker);
> + }
> bdrv_unref(bs->backing_hd);
> bs->backing_hd = NULL;
> }
Kevin
