[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 03/21] mirror: Error out when a BDS would get
From: |
Max Reitz |
Subject: |
Re: [Qemu-devel] [PATCH v2 03/21] mirror: Error out when a BDS would get two BBs |
Date: |
Fri, 27 Nov 2015 18:06:37 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 |
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 <address@hidden>
> ---
> 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_TIME 100000000ULL /* 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 <address@hidden>
> + 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