Re: [Qemu-devel] [RFC PATCH 08/11] qcow2: Add basic data-file infrastructure

[Max Reitz]

On 31.01.19 18:55, Kevin Wolf wrote:
> This adds a .bdrv_open option to specify the external data file node.
> 
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
>  qapi/block-core.json |  3 ++-
>  block/qcow2.h        |  4 +++-
>  block/qcow2.c        | 25 +++++++++++++++++++++++--
>  3 files changed, 28 insertions(+), 4 deletions(-)

[...]

> diff --git a/block/qcow2.h b/block/qcow2.h
> index c161970882..e2114900b4 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h

[...]

> @@ -205,7 +206,8 @@ enum {
>      QCOW2_INCOMPAT_DATA_FILE        = 1 << QCOW2_INCOMPAT_DATA_FILE_BITNR,
>  
>      QCOW2_INCOMPAT_MASK          = QCOW2_INCOMPAT_DIRTY
> -                                 | QCOW2_INCOMPAT_CORRUPT,
> +                                 | QCOW2_INCOMPAT_CORRUPT
> +                                 | QCOW2_INCOMPAT_DATA_FILE,

This hunk seems to belong somewhere else.

>  };
>  
>  /* Compatible feature bits */
> diff --git a/block/qcow2.c b/block/qcow2.c
> index ac9934b3ed..376232d3f0 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -1441,8 +1441,22 @@ static int coroutine_fn qcow2_do_open(BlockDriverState 
> *bs, QDict *options,
>          goto fail;
>      }
>  
> -    /* TODO Open external data file */
> -    s->data_file = bs->file;
> +    /* Open external data file */
> +    if (s->incompatible_features & QCOW2_INCOMPAT_DATA_FILE) {
> +        s->data_file = bdrv_open_child(NULL, options, "data-file", bs,
> +                                       &child_file, false, errp);
> +        if (!s->data_file) {
> +            ret = -EINVAL;
> +            goto fail;
> +        }
> +    } else if (qdict_get(options, QCOW2_OPT_DATA_FILE)) {

I get the idea, but this isn't crumpled so this key may not exist (but
data-file.driver and data-file.filename may).  Of course the fact that
these options remain unused will be caught by the block layer, but that
makes the error message below a bit less useful.

Max

> +        error_setg(errp, "'data-file' can only be set for images with an "
> +                         "external data file");
> +        ret = -EINVAL;
> +        goto fail;
> +    } else {
> +        s->data_file = bs->file;
> +    }
>  
>      /* qcow2_read_extension may have set up the crypto context
>       * if the crypt method needs a header region, some methods

signature.asc
Description: OpenPGP digital signature