[Qemu-devel] [PATCH v2 08/10] block: Allow changing 'discard' on reopen

Alberto Garcia posted 10 patches 7 years, 2 months ago
There is a newer version of this series
[Qemu-devel] [PATCH v2 08/10] block: Allow changing 'discard' on reopen
Posted by Alberto Garcia 7 years, 2 months ago
'discard' is one of the basic BlockdevOptions available for all
drivers, but it's not handled by bdrv_reopen_prepare() so any attempt
to change it results in an error:

   (qemu) qemu-io virtio0 "reopen -o discard=on"
   Cannot change the option 'discard'

Since there's no reason why we shouldn't allow changing it and the
implementation is simple let's just do it.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Cc: Max Reitz <mreitz@redhat.com>
---
 block.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/block.c b/block.c
index 1c0f72454a..3f4b7640fa 100644
--- a/block.c
+++ b/block.c
@@ -3155,6 +3155,7 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue,
     BlockDriver *drv;
     QemuOpts *opts;
     QDict *orig_reopen_opts;
+    const char *value;
     bool read_only;
 
     assert(reopen_state != NULL);
@@ -3177,6 +3178,15 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue,
 
     update_flags_from_options(&reopen_state->flags, opts);
 
+    value = qemu_opt_get_del(opts, "discard");
+    if (value != NULL) {
+        if (bdrv_parse_discard_flags(value, &reopen_state->flags) != 0) {
+            error_setg(errp, "Invalid discard option");
+            ret = -EINVAL;
+            goto error;
+        }
+    }
+
     /* All other options (including node-name and driver) must be unchanged.
      * Put them back into the QDict, so that they are checked at the end
      * of this function. */
-- 
2.11.0


Re: [Qemu-devel] [PATCH v2 08/10] block: Allow changing 'discard' on reopen
Posted by Max Reitz 7 years, 2 months ago
On 2018-09-03 16:34, Alberto Garcia wrote:
> 'discard' is one of the basic BlockdevOptions available for all
> drivers, but it's not handled by bdrv_reopen_prepare() so any attempt
> to change it results in an error:
> 
>    (qemu) qemu-io virtio0 "reopen -o discard=on"
>    Cannot change the option 'discard'
> 
> Since there's no reason why we shouldn't allow changing it and the
> implementation is simple let's just do it.
> 
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> Cc: Max Reitz <mreitz@redhat.com>
> ---
>  block.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)

Reviewed-by: Max Reitz <mreitz@redhat.com>

Re: [Qemu-devel] [PATCH v2 08/10] block: Allow changing 'discard' on reopen
Posted by Alberto Garcia 7 years, 2 months ago
On Mon 03 Sep 2018 04:34:06 PM CEST, Alberto Garcia <berto@igalia.com> wrote:

> +    value = qemu_opt_get_del(opts, "discard");
> +    if (value != NULL) {
> +        if (bdrv_parse_discard_flags(value, &reopen_state->flags) != 0) {
> +            error_setg(errp, "Invalid discard option");
> +            ret = -EINVAL;
> +            goto error;
> +        }
> +    }

I just realized that we're leaking the string here. qemu_opt_get_del()
expects that the returned value is freed by the caller.

I'll correct all the patches from this series with this problem and
resend the series.

Berto