Il 10/10/2012 07:56, Jeff Cody ha scritto:
> This simplifies some code and error checking, and also fixes a bug.
>
> bdrv_find_backing_image() should only be passed absolute filenames,
> or filenames relative to the chain. In the QMP message handler for
> block commit, when looking up the base do so from the determined top
> image, so we know it is reachable from top.
>
> Signed-off-by: Jeff Cody <jc...@redhat.com>
> ---
> block/commit.c | 9 ---------
> blockdev.c | 21 +++++++++++----------
> 2 files changed, 11 insertions(+), 19 deletions(-)
>
> diff --git a/block/commit.c b/block/commit.c
> index 733c914..13d9e82 100644
> --- a/block/commit.c
> +++ b/block/commit.c
> @@ -211,15 +211,6 @@ void commit_start(BlockDriverState *bs, BlockDriverState
> *base,
> return;
> }
>
> - /* top and base may be valid, but let's make sure that base is reachable
> - * from top */
> - if (bdrv_find_backing_image(top, base->filename) != base) {
> - error_setg(errp,
> - "Base (%s) is not reachable from top (%s)",
> - base->filename, top->filename);
> - return;
> - }
> -
> overlay_bs = bdrv_find_overlay(bs, top);
>
> if (overlay_bs == NULL) {
> diff --git a/blockdev.c b/blockdev.c
> index 99828ad..7052287 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1157,16 +1157,6 @@ void qmp_block_commit(const char *device,
> error_set(errp, QERR_DEVICE_NOT_FOUND, device);
> return;
> }
> - if (base && has_base) {
> - base_bs = bdrv_find_backing_image(bs, base);
> - } else {
> - base_bs = bdrv_find_base(bs);
> - }
> -
> - if (base_bs == NULL) {
> - error_set(errp, QERR_BASE_NOT_FOUND, base ? base : "NULL");
> - return;
> - }
>
> /* default top_bs is the active layer */
> top_bs = bs;
> @@ -1182,6 +1172,17 @@ void qmp_block_commit(const char *device,
> return;
> }
>
> + if (base && has_base) {
> + base_bs = bdrv_find_backing_image(top_bs, base);
> + } else {
> + base_bs = bdrv_find_base(top_bs);
> + }
> +
> + if (base_bs == NULL) {
> + error_set(errp, QERR_BASE_NOT_FOUND, base ? base : "NULL");
> + return;
> + }
> +
> commit_start(bs, base_bs, top_bs, speed, on_error, block_job_cb, bs,
> &local_err);
> if (local_err != NULL) {
>
This one looks good,
Reviewed-by: Paolo Bonzini <pbonz...@redhat.com>
Paolo
