Re: [Qemu-devel] [PATCH V9 3/4] block: Add backing file loop check in change_backing_file()

[Jeff Cody <jcody@redhat.com>]

On Tue, Nov 26, 2013 at 01:38:32AM -0500, Xu Wang wrote:
> Backing file loop should be checked before calling change_backing_
> file(). If loop appeared, this calling should be stopped and an
> error printed.
> 
> Signed-off-by: Xu Wang <gesaint@linux.vnet.ibm.com>
> ---
>  block.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/block.c b/block.c
> index b8cea1c..87f7018 100644
> --- a/block.c
> +++ b/block.c
> @@ -2075,6 +2075,7 @@ static void coroutine_fn wait_for_overlapping_requests(BlockDriverState *bs,
>   * Return values:
>   * 0        - success
>   * -EINVAL  - backing format specified, but no file
> + * -EIO     - generic I/O error (may happen for all errors)
>   * -ENOSPC  - can't update the backing file because no space is left in the
>   *            image file header
>   * -ENOTSUP - format driver doesn't support changing the backing file
> @@ -2083,6 +2084,7 @@ int bdrv_change_backing_file(BlockDriverState *bs,
>      const char *backing_file, const char *backing_fmt)
>  {
>      BlockDriver *drv = bs->drv;
> +    Error *local_err = NULL;
>      int ret;
>  
>      /* Backing file format doesn't make sense without a backing file */
> @@ -2090,6 +2092,13 @@ int bdrv_change_backing_file(BlockDriverState *bs,
>          return -EINVAL;
>      }
>  
> +    /* Check if loop exists in backing files chain after change */
> +    if (!bdrv_backing_chain_okay(backing_file, NULL, bs->filename, 
> +                                 &local_err)) {
> +        error_report("Backing file check: %s", error_get_pretty(local_err));

You need to free local_err before returning (i.e.
error_free(local_err)).



> +        return -EIO;
> +    }
> +
>      if (drv->bdrv_change_backing_file != NULL) {
>          ret = drv->bdrv_change_backing_file(bs, backing_file, backing_fmt);
>      } else {
> -- 
> 1.8.1.4
> 
>