Re: [Qemu-devel] [PATCH v2 01/37] blockdev: Allow creation of BDS trees without BB

[Kevin Wolf]

Am 04.03.2015 um 15:04 hat Max Reitz geschrieben:
> On 2015-03-04 at 08:39, Kevin Wolf wrote:
> >Am 09.02.2015 um 18:11 hat Max Reitz geschrieben:
> >>If the "id" field is missing from the options given to blockdev-add,
> >>just omit the BlockBackend and create the BlockDriverState tree alone.
> >>
> >>However, if "id" is missing, "node-name" must be specified; otherwise,
> >>the BDS tree would no longer be accessible.
> >>
> >>Signed-off-by: Max Reitz <address@hidden>
> >>---
> >>  blockdev.c                 | 44 
> >> +++++++++++++++++++++++++++++++-------------
> >>  qapi/block-core.json       | 13 +++++++++----
> >>  tests/qemu-iotests/087     |  2 +-
> >>  tests/qemu-iotests/087.out |  4 ++--
> >>  4 files changed, 43 insertions(+), 20 deletions(-)
> >>
> >>diff --git a/blockdev.c b/blockdev.c
> >>index 6eedcf5..6d67c80 100644
> >>--- a/blockdev.c
> >>+++ b/blockdev.c
> >>@@ -2822,17 +2822,12 @@ out:
> >>  void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
> >>  {
> >>      QmpOutputVisitor *ov = qmp_output_visitor_new();
> >>-    BlockBackend *blk;
> >>+    BlockDriverState *bs;
> >>+    BlockBackend *blk = NULL;
> >>      QObject *obj;
> >>      QDict *qdict;
> >>      Error *local_err = NULL;
> >>-    /* Require an ID in the top level */
> >>-    if (!options->has_id) {
> >>-        error_setg(errp, "Block device needs an ID");
> >>-        goto fail;
> >>-    }
> >>-
> >>      /* TODO Sort it out in raw-posix and drive_new(): Reject aio=native 
> >> with
> >>       * cache.direct=false instead of silently switching to aio=threads, 
> >> except
> >>       * when called from drive_new().
> >>@@ -2860,14 +2855,37 @@ void qmp_blockdev_add(BlockdevOptions *options, 
> >>Error **errp)
> >>      qdict_flatten(qdict);
> >>-    blk = blockdev_init(NULL, qdict, &local_err);
> >>-    if (local_err) {
> >>-        error_propagate(errp, local_err);
> >>-        goto fail;
> >>+    if (options->has_id) {
> >>+        blk = blockdev_init(NULL, qdict, &local_err);
> >>+        if (local_err) {
> >>+            error_propagate(errp, local_err);
> >>+            goto fail;
> >>+        }
> >>+
> >>+        bs = blk_bs(blk);
> >>+    } else {
> >>+        int ret;
> >>+
> >>+        if (!qdict_get_try_str(qdict, "node-name")) {
> >>+            error_setg(errp, "'id' and/or 'node-name' need to be specified 
> >>for "
> >>+                       "the root node");
> >>+            goto fail;
> >>+        }
> >>+
> >>+        bs = NULL;
> >>+        ret = bdrv_open(&bs, NULL, NULL, qdict, BDRV_O_RDWR | 
> >>BDRV_O_CACHE_WB,
> >>+                        NULL, errp);
> >Now all the qdict entries that aren't parsed by bdrv_open() but
> >converted into flags by blockdev_init() are broken if you don't give an
> >id. This includes read-only, discard, the cache options - in other
> >words, enough to make this "support" completely useless.
> 
> See patch 23.

No matter what I'll find there (okay, I've cheated and already quickly
looked at it before writing this), this answer tells me that you're
doing things in the wrong order.

Kevin