On 23.11.2015 16:59, Kevin Wolf wrote:
> bdrv_replace_in_backing_chain() asserts that not both old and new
> BlockDdriverState have a BlockBackend attached to them because both
> would have to end up pointing to the new BDS and we don't support more
> than one BB per BDS yet.
>
> Before we can safely allow references to existing nodes as backing
> files, we need to make sure that even if a backing file has a BB on it,
> this doesn't crash qemu.
>
> There are probably also some cases with the 'replaces' option set where
> drive-mirror could fail this assertion today. They are fixed with this
> error check as well.
>
> Signed-off-by: Kevin Wolf
> ---
> block/mirror.c | 28
> 1 file changed, 28 insertions(+)
>
> diff --git a/block/mirror.c b/block/mirror.c
> index 52c9abf..0620068 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -18,6 +18,7 @@
> #include "qapi/qmp/qerror.h"
> #include "qemu/ratelimit.h"
> #include "qemu/bitmap.h"
> +#include "qemu/error-report.h"
>
> #define SLICE_TIME1ULL /* ns */
> #define MAX_IN_FLIGHT 16
> @@ -370,11 +371,22 @@ static void mirror_exit(BlockJob *job, void *opaque)
> if (s->to_replace) {
> to_replace = s->to_replace;
> }
> +
> +/* This was checked in mirror_start_job(), but meanwhile one of the
> + * nodes could have been newly attached to a BlockBackend. */
> +if (to_replace->blk && s->target->blk) {
> +error_report("block job: Can't create node with two
> BlockBackends");
> +data->ret = -EINVAL;
> +goto out;
> +}
> +
> if (bdrv_get_flags(s->target) != bdrv_get_flags(to_replace)) {
> bdrv_reopen(s->target, bdrv_get_flags(to_replace), NULL);
> }
> bdrv_replace_in_backing_chain(to_replace, s->target);
> }
> +
> +out:
> if (s->to_replace) {
> bdrv_op_unblock_all(s->to_replace, s->replace_blocker);
> error_free(s->replace_blocker);
> @@ -701,6 +713,7 @@ static void mirror_start_job(BlockDriverState *bs,
> BlockDriverState *target,
> bool is_none_mode, BlockDriverState *base)
> {
> MirrorBlockJob *s;
> +BlockDriverState *replaced_bs;
>
> if (granularity == 0) {
> granularity = bdrv_get_default_bitmap_granularity(target);
> @@ -724,6 +737,21 @@ static void mirror_start_job(BlockDriverState *bs,
> BlockDriverState *target,
> buf_size = DEFAULT_MIRROR_BUF_SIZE;
> }
>
> +/* We can't support this case as long as the block layer can't handle
> + * multple BlockBackends per BlockDriverState. */
*multiple
With that fixed:
Reviewed-by: Max Reitz
> +if (replaces) {
> +replaced_bs = bdrv_lookup_bs(replaces, replaces, errp);
> +if (replaced_bs == NULL) {
> +return;
> +}
> +} else {
> +replaced_bs = bs;
> +}
> +if (replaced_bs->blk && target->blk) {
> +error_setg(errp, "Can't create node with two BlockBackends");
> +return;
> +}
> +
> s = block_job_create(driver, bs, speed, cb, opaque, errp);
> if (!s) {
> return;
>
signature.asc
Description: OpenPGP digital signature