Re: [Qemu-devel] [PATCH v2 21/45] block: add bdrv_open_backing_file

[Jeff Cody <jcody@redhat.com>]

On 09/26/2012 11:56 AM, Paolo Bonzini wrote:
> Mirroring runs without the backing file so that it can be copied outside
> QEMU.  However, we need to add it at the time the job is completed and
> QEMU switches to the target.  Factor out the common bits of opening an
> image and completing a mirroring operation.
> 
> The new function does not assume that the file is closed immediately after
> it returns failure, so it keeps the BDRV_O_NO_BACKING flag up-to-date.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>         v1->v2: do not close bs if the function fails (brain fart).
> 
>  block.c | 62 ++++++++++++++++++++++++++++++++++++++++++++------------------
>  block.h |  1 +
>  2 file modificati, 45 inserzioni(+), 18 rimozioni(-)
> 
> diff --git a/block.c b/block.c
> index 703261d..6ee7052 100644
> --- a/block.c
> +++ b/block.c
> @@ -734,6 +734,48 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename, int flags)
>      return 0;
>  }
>  
> +int bdrv_open_backing_file(BlockDriverState *bs)
> +{
> +    char backing_filename[PATH_MAX];
> +    int back_flags, ret;
> +    BlockDriver *back_drv = NULL;
> +
> +    if (bs->backing_hd != NULL) {
> +        return 0;
> +    }
> +
> +    bs->open_flags &= ~BDRV_O_NO_BACKING;
> +    if (bs->backing_file[0] == '\0') {
> +        return 0;
> +    }
> +
> +    bs->backing_hd = bdrv_new("");
> +    bdrv_get_full_backing_filename(bs, backing_filename,
> +                                   sizeof(backing_filename));
> +
> +    if (bs->backing_format[0] != '\0') {
> +        back_drv = bdrv_find_format(bs->backing_format);
> +    }
> +
> +    /* backing files always opened read-only */
> +    back_flags = bs->open_flags & ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT);
> +
> +    ret = bdrv_open(bs->backing_hd, backing_filename, back_flags, back_drv);
> +    if (ret < 0) {
> +        bdrv_delete(bs->backing_hd);
> +        bs->backing_hd = NULL;
> +        bs->open_flags |= BDRV_O_NO_BACKING;
> +        return ret;
> +    }
> +    if (bs->is_temporary) {
> +        bs->backing_hd->keep_read_only = !(bs->open_flags & BDRV_O_RDWR);
> +    } else {
> +        /* base images use the same setting as leaf */
> +        bs->backing_hd->keep_read_only = bs->keep_read_only;
> +    }

The bs->keep_read_only flag no longer exists.  I think you can safely delete
the above 6 lines... BDRV_O_ALLOW_RDWR is now used instead, and will be
pulled in from bs->open_flags.  (see commit be028ad and dc1c13d).

> +    return 0;
> +}
> +
>  /*
>   * Opens a disk image (raw, qcow2, vmdk, ...)
>   */
> @@ -821,24 +863,8 @@ int bdrv_open(BlockDriverState *bs, const char *filename, int flags,
>      }
>  
>      /* If there is a backing file, use it */
> -    if ((flags & BDRV_O_NO_BACKING) == 0 && bs->backing_file[0] != '\0') {
> -        char backing_filename[PATH_MAX];
> -        int back_flags;
> -        BlockDriver *back_drv = NULL;
> -
> -        bs->backing_hd = bdrv_new("");
> -        bdrv_get_full_backing_filename(bs, backing_filename,
> -                                       sizeof(backing_filename));
> -
> -        if (bs->backing_format[0] != '\0') {
> -            back_drv = bdrv_find_format(bs->backing_format);
> -        }
> -
> -        /* backing files always opened read-only */
> -        back_flags =
> -            flags & ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);
> -
> -        ret = bdrv_open(bs->backing_hd, backing_filename, back_flags, back_drv);
> +    if ((flags & BDRV_O_NO_BACKING) == 0) {
> +        ret = bdrv_open_backing_file(bs);
>          if (ret < 0) {
>              bdrv_close(bs);
>              return ret;
> diff --git a/block.h b/block.h
> index aa1121a..08479e1 100644
> --- a/block.h
> +++ b/block.h
> @@ -133,6 +133,7 @@ void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top);
>  void bdrv_delete(BlockDriverState *bs);
>  int bdrv_parse_cache_flags(const char *mode, int *flags);
>  int bdrv_file_open(BlockDriverState **pbs, const char *filename, int flags);
> +int bdrv_open_backing_file(BlockDriverState *bs);
>  int bdrv_open(BlockDriverState *bs, const char *filename, int flags,
>                BlockDriver *drv);
>  BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
>