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

Alberto Garcia posted 13 patches 6 years, 9 months ago
Maintainers: Max Reitz <mreitz@redhat.com>, Liu Yuan <namei.unix@gmail.com>, Josh Durgin <jdurgin@redhat.com>, Wen Congyang <wencongyang2@huawei.com>, Jeff Cody <jcody@redhat.com>, Peter Lieven <pl@kamp.de>, Fam Zheng <fam@euphon.net>, Xie Changlong <xiechanglong.d@gmail.com>, Ronnie Sahlberg <ronniesahlberg@gmail.com>, Kevin Wolf <kwolf@redhat.com>, Markus Armbruster <armbru@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Eric Blake <eblake@redhat.com>
There is a newer version of this series
[Qemu-devel] [PATCH 07/13] block: Allow omitting the 'backing' option in certain cases
Posted by Alberto Garcia 6 years, 9 months ago
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 <berto@igalia.com>
---
 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;
-- 
2.11.0


Re: [Qemu-devel] [PATCH 07/13] block: Allow omitting the 'backing' option in certain cases
Posted by Kevin Wolf 6 years, 8 months ago
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 <berto@igalia.com>
> ---
>  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