Re: [Qemu-devel] [PATCH v4 5/7] commit: support commit active layer

[Paolo Bonzini <pbonzini@redhat.com>]

Il 30/09/2013 14:02, Fam Zheng ha scritto:
> +        /* Mirror code already swapped bs and base, we drop the bs loop chain
> +         * formed by the swap: break the loop chain, trigger the chain unref.
> +         */
> +        p = info->base->backing_hd;
> +        info->base->backing_hd = NULL;
> +        bdrv_unref(p);

Should this code be in mirror_run? Perhaps extracting it into a new
function together with the preceding lines:

        if (bdrv_get_flags(s->target) != bdrv_get_flags(s->common.bs)) {
            bdrv_reopen(s->target, bdrv_get_flags(s->common.bs), NULL);
        }
        bdrv_swap(s->target, s->common.bs);

Same for this:

> +        if (bdrv_reopen(base_bs, bs->open_flags, &local_err)) {
> +            error_propagate(errp, local_err);
> +            return;
> +        }
> +        bdrv_ref(base_bs);

Perhaps you can have an internal function mirror_start_job and two
front-ends mirror_start + commit_active_start that wrap it.  Similarly,
mirror_start_job can take the base BDS and a bool instead of
MirrorSyncMode (removing the need for MIRROR_SYNC_MODE_COMMON).

But apart from these details, the series looks very nice.

Paolo