[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 07/34] block: Move flag inheritance to bdrv_open
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH 07/34] block: Move flag inheritance to bdrv_open_inherited() |
Date: |
Tue, 12 May 2015 15:32:57 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 11.05.2015 um 17:20 hat Max Reitz geschrieben:
> On 08.05.2015 19:21, Kevin Wolf wrote:
> >Instead of letting every caller of bdrv_open() determine the right flags
> >for its child node manually and pass them to the function, pass the
> >parent node and the role of the newly opened child (like backing file,
> >protocol layer, etc.).
> >
> >Signed-off-by: Kevin Wolf <address@hidden>
> >---
> > block.c | 74
> > ++++++++++++++++++++++++++++++++++++++---------
> > block/blkdebug.c | 2 +-
> > block/blkverify.c | 4 +--
> > block/quorum.c | 4 +--
> > block/vmdk.c | 5 ++--
> > include/block/block.h | 4 ++-
> > include/block/block_int.h | 7 +++++
> > 7 files changed, 78 insertions(+), 22 deletions(-)
> >
> >diff --git a/block.c b/block.c
> >index cea022f..c4f0fb4 100644
> >--- a/block.c
> >+++ b/block.c
> >@@ -79,6 +79,12 @@ static QTAILQ_HEAD(, BlockDriverState) graph_bdrv_states =
> > static QLIST_HEAD(, BlockDriver) bdrv_drivers =
> > QLIST_HEAD_INITIALIZER(bdrv_drivers);
> >+static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,
> >+ const char *reference, QDict *options, int
> >flags,
> >+ BlockDriverState* parent,
>
> Stern zur Variable!
Okay, now that we're in nitpicking mode:
Zur Variablen, wenn schon.
(Sorry for starting a discussion about German grammar on qemu-devel, but
I just have to...)
> >+ const BdrvChildRole *child_role,
> >+ BlockDriver *drv, Error **errp);
> >+
> > static void bdrv_dirty_bitmap_truncate(BlockDriverState *bs);
> > /* If non-zero, use only whitelisted block drivers */
> > static int use_bdrv_whitelist;
> >@@ -672,8 +678,8 @@ static int bdrv_temp_snapshot_flags(int flags)
> > }
> > /*
> >- * Returns the flags that bs->file should get, based on the given flags for
> >- * the parent BDS
> >+ * Returns the flags that bs->file should get if a protocol driver is
> >expected,
> >+ * based on the given flags for the parent BDS
> > */
> > static int bdrv_inherited_flags(int flags)
> > {
> >@@ -690,6 +696,25 @@ static int bdrv_inherited_flags(int flags)
> > return flags;
> > }
> >+const BdrvChildRole child_file = {
> >+ .inherit_flags = bdrv_inherited_flags,
> >+};
> >+
> >+/*
> >+ * Returns the flags that bs->file should get if the use of formats (and not
> >+ * only protocols) is permitted for it, based on the given flags for the
> >parent
> >+ * BDS
> >+ */
> >+static int bdrv_inherited_fmt_flags(int parent_flags)
> >+{
> >+ int flags = child_file.inherit_flags(parent_flags);
> >+ return flags & ~BDRV_O_PROTOCOL;
> >+}
> >+
> >+const BdrvChildRole child_format = {
> >+ .inherit_flags = bdrv_inherited_fmt_flags,
> >+};
> >+
> > /*
> > * Returns the flags that bs->backing_hd should get, based on the given
> > flags
> > * for the parent BDS
> >@@ -705,6 +730,10 @@ static int bdrv_backing_flags(int flags)
> > return flags;
> > }
> >+static const BdrvChildRole child_backing = {
> >+ .inherit_flags = bdrv_backing_flags,
> >+};
> >+
> > static int bdrv_open_flags(BlockDriverState *bs, int flags)
> > {
> > int open_flags = flags | BDRV_O_CACHE_WB;
> >@@ -827,7 +856,6 @@ static int bdrv_open_common(BlockDriverState *bs,
> >BlockDriverState *file,
> > return 0;
> > }
> >- bs->open_flags = flags;
> > bs->guest_block_size = 512;
> > bs->request_alignment = 512;
> > bs->zero_beyond_eof = true;
> >@@ -1134,9 +1162,10 @@ int bdrv_open_backing_file(BlockDriverState *bs,
> >QDict *options, Error **errp)
> > }
> > assert(bs->backing_hd == NULL);
> >- ret = bdrv_open(&backing_hd,
> >- *backing_filename ? backing_filename : NULL, NULL,
> >options,
> >- bdrv_backing_flags(bs->open_flags), NULL, &local_err);
> >+ ret = bdrv_open_inherit(&backing_hd,
> >+ *backing_filename ? backing_filename : NULL,
> >+ NULL, options, 0, bs, &child_backing,
> >+ NULL, &local_err);
> > if (ret < 0) {
> > bdrv_unref(backing_hd);
> > backing_hd = NULL;
> >@@ -1170,7 +1199,8 @@ free_exit:
> > * To conform with the behavior of bdrv_open(), *pbs has to be NULL.
> > */
> > int bdrv_open_image(BlockDriverState **pbs, const char *filename,
> >- QDict *options, const char *bdref_key, int flags,
> >+ QDict *options, const char *bdref_key,
> >+ BlockDriverState* parent, const BdrvChildRole
> >*child_role,
>
> !
>
> > bool allow_none, Error **errp)
> > {
> > QDict *image_options;
> >@@ -1198,7 +1228,8 @@ int bdrv_open_image(BlockDriverState **pbs, const char
> >*filename,
> > goto done;
> > }
> >- ret = bdrv_open(pbs, filename, reference, image_options, flags, NULL,
> >errp);
> >+ ret = bdrv_open_inherit(pbs, filename, reference, image_options, 0,
> >+ parent, child_role, NULL, errp);
> > done:
> > qdict_del(options, bdref_key);
> >@@ -1285,9 +1316,11 @@ out:
> > * should be opened. If specified, neither options nor a filename may be
> > given,
> > * nor can an existing BDS be reused (that is, *pbs has to be NULL).
> > */
> >-int bdrv_open(BlockDriverState **pbs, const char *filename,
> >- const char *reference, QDict *options, int flags,
> >- BlockDriver *drv, Error **errp)
> >+static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,
> >+ const char *reference, QDict *options, int
> >flags,
> >+ BlockDriverState* parent,
>
> Faithfully following the forward declaration, so the same issue here.
>
> Other than these nitpicks:
>
> Reviewed-by: Max Reitz <address@hidden>
Thanks, will fix.
Kevin
- Re: [Qemu-devel] [PATCH 06/34] block: Use QemuOpts in bdrv_open_common(), (continued)
- [Qemu-devel] [PATCH 09/34] block: Add BlockDriverState.inherits_from, Kevin Wolf, 2015/05/08
- [Qemu-devel] [PATCH 10/34] block: Fix reopen flag inheritance, Kevin Wolf, 2015/05/08
- [Qemu-devel] [PATCH 07/34] block: Move flag inheritance to bdrv_open_inherited(), Kevin Wolf, 2015/05/08
- [Qemu-devel] [PATCH 08/34] block: Add list of children to BlockDriverState, Kevin Wolf, 2015/05/08
- [Qemu-devel] [PATCH 11/34] block: Allow references for backing files, Kevin Wolf, 2015/05/08