| Message ID | a54f07d612171ce5c3994373356447d0c6df0679.1541595424.git.berto@igalia.com |
|---|---|
| State | New |
| Headers | show |
| Series | Don't pass flags to bdrv_reopen_queue() | expand |
On 07.11.18 13:59, Alberto Garcia wrote: > Towards the end of bdrv_reopen_queue_child(), before starting to > process the children, the update_flags_from_options() function is > called in order to have BDRVReopenState.flags in sync with the options > from the QDict. > > This is necessary because during the reopen process flags must be > updated for all nodes in the queue so bdrv_is_writable_after_reopen() > and the permission checks work correctly. > > Because of that, calling update_flags_from_options() again in > bdrv_reopen_prepare() doesn't really change the flags (they are > already up-to-date). But we need to call it in order to remove those > options from QemuOpts and that way indicate that they have been > processed. > > Signed-off-by: Alberto Garcia <berto@igalia.com> > --- > block.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/block.c b/block.c > index 68f1e3b45e..03277b3d19 100644 > --- a/block.c > +++ b/block.c > @@ -3178,6 +3178,7 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue, > Error **errp) > { > int ret = -1; > + int old_flags; > Error *local_err = NULL; > BlockDriver *drv; > QemuOpts *opts; > @@ -3203,7 +3204,12 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue, > goto error; > } > > + /* This was already called in bdrv_reopen_queue_child() so the flags > + * are up-to-date. This time we simply want to remove the options from > + * QemuOpts in order to indicate that they have been processed. */ > + old_flags = reopen_state->flags; > update_flags_from_options(&reopen_state->flags, opts); > + assert(old_flags == reopen_state->flags); Reviewed-by: Max Reitz <mreitz@redhat.com> Although as my bike-shedding for today I'd like to say that I'd find it more intuitive to store the "just remove the options" call result into old_flags instead (or rather something renamed), i.e. flags_copy = reopen_state->flags; update_flags_from_options(&flags_copy, opts); assert(flags_copy == reopen_state->flags); Not that it matters. Max
diff --git a/block.c b/block.c index 68f1e3b45e..03277b3d19 100644 --- a/block.c +++ b/block.c @@ -3178,6 +3178,7 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue, Error **errp) { int ret = -1; + int old_flags; Error *local_err = NULL; BlockDriver *drv; QemuOpts *opts; @@ -3203,7 +3204,12 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue, goto error; } + /* This was already called in bdrv_reopen_queue_child() so the flags + * are up-to-date. This time we simply want to remove the options from + * QemuOpts in order to indicate that they have been processed. */ + old_flags = reopen_state->flags; update_flags_from_options(&reopen_state->flags, opts); + assert(old_flags == reopen_state->flags); discard = qemu_opt_get_del(opts, BDRV_OPT_DISCARD); if (discard != NULL) {
Towards the end of bdrv_reopen_queue_child(), before starting to process the children, the update_flags_from_options() function is called in order to have BDRVReopenState.flags in sync with the options from the QDict. This is necessary because during the reopen process flags must be updated for all nodes in the queue so bdrv_is_writable_after_reopen() and the permission checks work correctly. Because of that, calling update_flags_from_options() again in bdrv_reopen_prepare() doesn't really change the flags (they are already up-to-date). But we need to call it in order to remove those options from QemuOpts and that way indicate that they have been processed. Signed-off-by: Alberto Garcia <berto@igalia.com> --- block.c | 6 ++++++ 1 file changed, 6 insertions(+)