On 04/05/2017 02:28 PM, Jeff Cody wrote:
> Signed-off-by: Jeff Cody <[email protected]>
> ---
> block.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/block.c b/block.c
> index ad958b9..3245fae 100644
> --- a/block.c
> +++ b/block.c
> @@ -2785,6 +2785,7 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state,
> BlockReopenQueue *queue,
> BlockDriver *drv;
> QemuOpts *opts;
> const char *value;
> + bool read_only;
>
> assert(reopen_state != NULL);
> assert(reopen_state->bs->drv != NULL);
> @@ -2813,12 +2814,13 @@ int bdrv_reopen_prepare(BDRVReopenState
> *reopen_state, BlockReopenQueue *queue,
> qdict_put(reopen_state->options, "driver", qstring_from_str(value));
> }
>
> - /* if we are to stay read-only, do not allow permission change
> - * to r/w */
> - if (!(reopen_state->bs->open_flags & BDRV_O_ALLOW_RDWR) &&
> - reopen_state->flags & BDRV_O_RDWR) {
So the current code checks reopen_state->flags & BDRV_O_RDWR;
> - error_setg(errp, "Node '%s' is read only",
> - bdrv_get_device_or_node_name(reopen_state->bs));
> + /* If we are to stay read-only, do not allow permission change
> + * to r/w. Attempting to set to r/w may fail if either BDRV_O_ALLOW_RDWR
> is
> + * not set, or if the BDS still has copy_on_read enabled */
> + read_only = !(reopen_state->bs->open_flags & BDRV_O_RDWR);
And the proposed change checks reopen_state->bs->open_flags &
BDRV_O_RDWR. (It's negated again inside of bdrv_try_set_read_only.)
Both check against !(reopen_state->bs->open_flags & BDRV_O_ALLOW_RDWR).
What's the functional difference of doing so, and is it intentional?
> + ret = bdrv_try_set_read_only(reopen_state->bs, read_only, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> goto error;
> }
>
>
