From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([209.51.188.92]:37157) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gtb5N-0007hY-5l for qemu-devel@nongnu.org; Tue, 12 Feb 2019 11:39:06 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gtb5L-0005ld-Nu for qemu-devel@nongnu.org; Tue, 12 Feb 2019 11:39:04 -0500 Date: Tue, 12 Feb 2019 17:38:53 +0100 From: Kevin Wolf Message-ID: <20190212163853.GG5283@localhost.localdomain> References: <6ec504767392cb91591542c416f0c2ae52f847f8.1547739122.git.berto@igalia.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <6ec504767392cb91591542c416f0c2ae52f847f8.1547739122.git.berto@igalia.com> Subject: Re: [Qemu-devel] [PATCH 07/13] block: Allow omitting the 'backing' option in certain cases List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alberto Garcia Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org, Max Reitz 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 > --- > 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