qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PULL 41/56] block/copy-before-write: make public block driver


From: Vladimir Sementsov-Ogievskiy
Subject: Re: [PULL 41/56] block/copy-before-write: make public block driver
Date: Mon, 20 Sep 2021 13:08:14 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.14.0

20.09.2021 12:41, Kevin Wolf wrote:
Am 01.09.2021 um 17:16 hat Hanna Reitz geschrieben:
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Finally, copy-before-write gets own .bdrv_open and .bdrv_close
handlers, block_init() call and becomes available through bdrv_open().

To achieve this:

  - cbw_init gets unused flags argument and becomes cbw_open
  - block_copy_state_free() call moved to new cbw_close()
  - in bdrv_cbw_append:
    - options are completed with driver and node-name, and we can simply
      use bdrv_insert_node() to do both open and drained replacing
  - in bdrv_cbw_drop:
    - cbw_close() is now responsible for freeing s->bcs, so don't do it
      here

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20210824083856.17408-22-vsementsov@virtuozzo.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>

This patch results in a change in behaviour that I assume was not
intentional: Previously, creating a backup block job would succeed to
insert the filter node independently of whether the filter driver was
whitelisted or not. After this patch, it becomes an error if the filter
driver isn't explicitly whitelisted:

https://bugzilla.redhat.com/show_bug.cgi?id=2004812

I'm sorry :( Will try to fix that today.

[..]

-    ret = bdrv_replace_node(source, top, errp);
-    bdrv_drained_end(source);
-    if (ret < 0) {
-        error_prepend(errp, "Cannot append copy-before-write filter: ");
-        goto fail;
+    top = bdrv_insert_node(source, opts, BDRV_O_RDWR, errp);

On the other hand, bdrv_insert_node() uses bdrv_open() internally, which
is a really long and messy code path that basically has to handle all of
the craziness that compatibility code for -drive needs to have.

Do we really need bdrv_open() here?

Or is all you really need just a version of bdrv_new_open_driver() that
accepts an options QDict instead of just flags?


Yes something like this, to always go through same interface.


--
Best regards,
Vladimir



reply via email to

[Prev in Thread] Current Thread [Next in Thread]