Re: [Qemu-block] [PATCH 07/13] block: Allow omitting the 'backing' option in certain cases

[Kevin Wolf]

Am 17.01.2019 um 16:33 hat Alberto Garcia geschrieben:
> Of all options of type BlockdevRef used to specify children in
> BlockdevOptions, 'backing' is the only one that is optional.
> 
> For "x-blockdev-reopen" we want that if an option is omitted then it
> must be reset to its default value. The default value of 'backing'
> means that QEMU opens the backing file specified in the image
> metadata, but this is not something that we want to support for the
> reopen operation.
> 
> Because of this the 'backing' option has to be specified during
> reopen, pointing to the existing backing file if we want to keep it,
> or pointing to a different one (or NULL) if we want to replace it (to
> be implemented in a subsequent patch).
> 
> In order to simplify things a bit and not to require that the user
> passes the 'backing' option to every single block device even when
> it's clearly not necessary, this patch allows omitting this option if
> the block device being reopened doesn't have a backing file attached
> _and_ no default backing file is specified in the image metadata.
> 
> Signed-off-by: Alberto Garcia <address@hidden>
> ---
>  block.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/block.c b/block.c
> index fd51f1cd35..897c8b85cd 100644
> --- a/block.c
> +++ b/block.c
> @@ -3336,7 +3336,13 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, 
> BlockReopenQueue *queue,
>  
>      drv_prepared = true;
>  
> -    if (reopen_state->backing_missing) {
> +    /*
> +     * We must provide the 'backing' option if the BDS has a backing
> +     * file or if the image file has a backing file name as part of
> +     * its metadata. Otherwise the 'backing' option can be omitted.
> +     */
> +    if (reopen_state->backing_missing &&
> +        (backing_bs(reopen_state->bs) || reopen_state->bs->backing_file[0])) 
> {
>          error_setg(errp, "backing is missing for '%s'",
>                     reopen_state->bs->node_name);
>          ret = -EINVAL;

Okay, this should be enough to fix drivers without support for backing
files again. Might be worth mentioning in the commit message of this and
the previous patch.

Normally, I would suggest merging this into the previous patch to keep
things bisectable, but keep_old_opts == false isn't used yet, so this
isn't a concern in this case.

Kevin