Re: [Qemu-devel] [PATCH v7 for-2.12 20/25] block: Generically refresh runtime options

From: Alberto Garcia <berto@igalia.com>
To: Max Reitz <mreitz@redhat.com>, qemu-block@nongnu.org
Cc: qemu-devel@nongnu.org, Kevin Wolf <kwolf@redhat.com>,
	Eric Blake <eblake@redhat.com>, John Snow <jsnow@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v7 for-2.12 20/25] block: Generically refresh runtime options
Date: Mon, 04 Dec 2017 17:16:32 +0100	[thread overview]
Message-ID: <w51wp2231un.fsf@maestria.local.igalia.com> (raw)
In-Reply-To: <20171120201004.14999-21-mreitz@redhat.com>

On Mon 20 Nov 2017 09:09:59 PM CET, Max Reitz wrote:
> Instead of having every block driver which implements
> bdrv_refresh_filename() copy all of the significant runtime options over
> to bs->full_open_options, implement this process generically in
> bdrv_refresh_filename().
>
> This patch only adds this new generic implementation, it does not remove
> the old functionality. This is done in a follow-up patch.
>
> With this patch, some superfluous information (that should never have
> been there) may be removed from some JSON filenames, as can be seen in
> the change to iotest 110's reference output.  In case of 191, backing
> nodes that have not been overridden are now removed from the filename.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>

> +static const char *const *significant_options(BlockDriverState *bs,
> +                                              const char *const *curopt)
> +{
> +    static const char *const global_options[] = {
> +        "driver", "filename", "base-directory", NULL
> +    };

You forgot to remove "base-directory" here from v6, but else the patch
looks good.

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto