block: Use bdrv_inherited_options()

Let child_file's, child_format's, and child_backing's .inherit_options()
implementations fall back to bdrv_inherited_options() to show that it
would really work for all of these cases, if only the parents passed the
appropriate BdrvChildRole and parent_is_format values.

(Also, make bdrv_open_inherit(), the only place to explicitly call
bdrv_backing_options(), call bdrv_inherited_options() instead.)

This patch should incur only two visible changes, both for child_format
children, both of which are effectively bug fixes:

First, they no longer have discard=unmap set by default.  This reason it
was set is because bdrv_inherited_fmt_options() fell through to
bdrv_protocol_options(), and that set it because "format drivers take
care to send flushes and respect unmap policy".  None of the drivers
that use child_format for their children (quorum and blkverify) are
format drivers, though, so this reasoning does not apply here.

Second, they no longer have BDRV_O_NO_IO force-cleared.  child_format
was used solely for children that do not store any metadata and as such
will not be accessed by their parents as long as those parents do not
receive I/O themselves.  Thus, such children should inherit
BDRV_O_NO_IO.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200513110544.176672-12-mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
This commit is contained in:
Max Reitz 2020-05-13 13:05:21 +02:00 committed by Kevin Wolf
parent fae8bd3904
commit 00ff7ffd67

63
block.c
View File

@ -80,6 +80,11 @@ static BlockDriverState *bdrv_open_inherit(const char *filename,
BdrvChildRole child_role,
Error **errp);
/* TODO: Remove when no longer needed */
static void bdrv_inherited_options(BdrvChildRole role, bool parent_is_format,
int *child_flags, QDict *child_options,
int parent_flags, QDict *parent_options);
/* If non-zero, use only whitelisted block drivers */
static int use_bdrv_whitelist;
@ -1158,31 +1163,9 @@ static void bdrv_protocol_options(BdrvChildRole role, bool parent_is_format,
int *child_flags, QDict *child_options,
int parent_flags, QDict *parent_options)
{
int flags = parent_flags;
/* Enable protocol handling, disable format probing for bs->file */
flags |= BDRV_O_PROTOCOL;
/* If the cache mode isn't explicitly set, inherit direct and no-flush from
* the parent. */
qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_DIRECT);
qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_NO_FLUSH);
qdict_copy_default(child_options, parent_options, BDRV_OPT_FORCE_SHARE);
/* Inherit the read-only option from the parent if it's not set */
qdict_copy_default(child_options, parent_options, BDRV_OPT_READ_ONLY);
qdict_copy_default(child_options, parent_options, BDRV_OPT_AUTO_READ_ONLY);
/* Our block drivers take care to send flushes and respect unmap policy,
* so we can default to enable both on lower layers regardless of the
* corresponding parent options. */
qdict_set_default_str(child_options, BDRV_OPT_DISCARD, "unmap");
/* Clear flags that only apply to the top layer */
flags &= ~(BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING | BDRV_O_COPY_ON_READ |
BDRV_O_NO_IO);
*child_flags = flags;
bdrv_inherited_options(BDRV_CHILD_IMAGE, true,
child_flags, child_options,
parent_flags, parent_options);
}
const BdrvChildClass child_file = {
@ -1209,11 +1192,9 @@ static void bdrv_inherited_fmt_options(BdrvChildRole role,
int *child_flags, QDict *child_options,
int parent_flags, QDict *parent_options)
{
child_file.inherit_options(role, parent_is_format,
bdrv_inherited_options(BDRV_CHILD_DATA, false,
child_flags, child_options,
parent_flags, parent_options);
*child_flags &= ~(BDRV_O_PROTOCOL | BDRV_O_NO_IO);
}
const BdrvChildClass child_format = {
@ -1295,23 +1276,9 @@ static void bdrv_backing_options(BdrvChildRole role, bool parent_is_format,
int *child_flags, QDict *child_options,
int parent_flags, QDict *parent_options)
{
int flags = parent_flags;
/* The cache mode is inherited unmodified for backing files; except WCE,
* which is only applied on the top level (BlockBackend) */
qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_DIRECT);
qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_NO_FLUSH);
qdict_copy_default(child_options, parent_options, BDRV_OPT_FORCE_SHARE);
/* backing files always opened read-only */
qdict_set_default_str(child_options, BDRV_OPT_READ_ONLY, "on");
qdict_set_default_str(child_options, BDRV_OPT_AUTO_READ_ONLY, "off");
flags &= ~BDRV_O_COPY_ON_READ;
/* snapshot=on is handled on the top layer */
flags &= ~(BDRV_O_SNAPSHOT | BDRV_O_TEMPORARY);
*child_flags = flags;
bdrv_inherited_options(BDRV_CHILD_COW, true,
child_flags, child_options,
parent_flags, parent_options);
}
static int bdrv_backing_update_filename(BdrvChild *c, BlockDriverState *base,
@ -1360,8 +1327,7 @@ const BdrvChildClass child_backing = {
* Returns the options and flags that a generic child of a BDS should
* get, based on the given options and flags for the parent BDS.
*/
static void __attribute__((unused))
bdrv_inherited_options(BdrvChildRole role, bool parent_is_format,
static void bdrv_inherited_options(BdrvChildRole role, bool parent_is_format,
int *child_flags, QDict *child_options,
int parent_flags, QDict *parent_options)
{
@ -3300,7 +3266,8 @@ static BlockDriverState *bdrv_open_inherit(const char *filename,
flags, options);
/* Let bdrv_backing_options() override "read-only" */
qdict_del(options, BDRV_OPT_READ_ONLY);
bdrv_backing_options(0, true, &flags, options, flags, options);
bdrv_inherited_options(BDRV_CHILD_COW, true,
&flags, options, flags, options);
}
bs->open_flags = flags;