All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] block: remove format defaults from QemuOpts in bdrv_create_file()
@ 2021-03-08 16:12 Stefano Garzarella
  2021-03-09 11:05 ` Kevin Wolf
  0 siblings, 1 reply; 2+ messages in thread
From: Stefano Garzarella @ 2021-03-08 16:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, qemu-block, Max Reitz

QemuOpts is usually created merging the QemuOptsList of format
and protocol. So, when the format calls bdr_create_file(), the 'opts'
parameter contains a QemuOptsList with a combination of format and
protocol default values.

The format properly removes its options before calling
bdr_create_file(), but the default values remain in 'opts->list'.
So if the protocol has options with the same name (e.g. rbd has
'cluster_size' as qcow2), it will see the default values of the format,
since for overlapping options, the format wins.

To avoid this issue, lets convert QemuOpts to QDict, in this way we take
only the set options, and then convert it back to QemuOpts, using the
'create_opts' of the protocol. So the new QemuOpts, will contain only the
protocol defaults.

Suggested-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 block.c | 36 +++++++++++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index a1f3cecd75..be7083c7d8 100644
--- a/block.c
+++ b/block.c
@@ -670,14 +670,48 @@ out:
 
 int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp)
 {
+    QemuOpts *protocol_opts;
     BlockDriver *drv;
+    QDict *qdict;
+    int ret;
 
     drv = bdrv_find_protocol(filename, true, errp);
     if (drv == NULL) {
         return -ENOENT;
     }
 
-    return bdrv_create(drv, filename, opts, errp);
+    if (!drv->create_opts) {
+        error_setg(errp, "Driver '%s' does not support image creation",
+                   drv->format_name);
+        return -ENOTSUP;
+    }
+
+    /*
+     * 'opts' contains a QemuOptsList with a combination of format and protocol
+     * default values.
+     *
+     * The format properly removes its options, but the default values remain
+     * in 'opts->list'.  So if the protocol has options with the same name
+     * (e.g. rbd has 'cluster_size' as qcow2), it will see the default values
+     * of the format, since for overlapping options, the format wins.
+     *
+     * To avoid this issue, lets convert QemuOpts to QDict, in this way we take
+     * only the set options, and then convert it back to QemuOpts, using the
+     * create_opts of the protocol. So the new QemuOpts, will contain only the
+     * protocol defaults.
+     */
+    qdict = qemu_opts_to_qdict(opts, NULL);
+    protocol_opts = qemu_opts_from_qdict(drv->create_opts, qdict, errp);
+    if (protocol_opts == NULL) {
+        ret = -EINVAL;
+        goto out;
+    }
+
+    ret = bdrv_create(drv, filename, protocol_opts, errp);
+out:
+    qemu_opts_del(protocol_opts);
+    qobject_unref(qdict);
+    return ret;
 }
 
 int coroutine_fn bdrv_co_delete_file(BlockDriverState *bs, Error **errp)
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] block: remove format defaults from QemuOpts in bdrv_create_file()
  2021-03-08 16:12 [PATCH] block: remove format defaults from QemuOpts in bdrv_create_file() Stefano Garzarella
@ 2021-03-09 11:05 ` Kevin Wolf
  0 siblings, 0 replies; 2+ messages in thread
From: Kevin Wolf @ 2021-03-09 11:05 UTC (permalink / raw)
  To: Stefano Garzarella; +Cc: qemu-devel, qemu-block, Max Reitz

Am 08.03.2021 um 17:12 hat Stefano Garzarella geschrieben:
> QemuOpts is usually created merging the QemuOptsList of format
> and protocol. So, when the format calls bdr_create_file(), the 'opts'
> parameter contains a QemuOptsList with a combination of format and
> protocol default values.
> 
> The format properly removes its options before calling
> bdr_create_file(), but the default values remain in 'opts->list'.
> So if the protocol has options with the same name (e.g. rbd has
> 'cluster_size' as qcow2), it will see the default values of the format,
> since for overlapping options, the format wins.
> 
> To avoid this issue, lets convert QemuOpts to QDict, in this way we take
> only the set options, and then convert it back to QemuOpts, using the
> 'create_opts' of the protocol. So the new QemuOpts, will contain only the
> protocol defaults.
> 
> Suggested-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>

Thanks, applied to the block branch.

Kevin



^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-03-09 11:07 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-08 16:12 [PATCH] block: remove format defaults from QemuOpts in bdrv_create_file() Stefano Garzarella
2021-03-09 11:05 ` Kevin Wolf

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.