Re: [RFC PATCH v2 3/4] block: Support multiple reopening with x-blockdev-reopen

2021-03-01 Thread Peter Krempa
On Mon, Mar 01, 2021 at 12:07:26 +0100, Kevin Wolf wrote:
> Am 25.02.2021 um 18:02 hat Vladimir Sementsov-Ogievskiy geschrieben:
> > 24.02.2021 15:33, Kevin Wolf wrote:
> > > Am 09.02.2021 um 09:03 hat Vladimir Sementsov-Ogievskiy geschrieben:
> > > > 08.02.2021 21:44, Alberto Garcia wrote:
> > > > > Signed-off-by: Alberto Garcia 
> > > > > ---
> > > > >qapi/block-core.json   |  2 +-
> > > > >include/block/block.h  |  1 +
> > > > >block.c| 16 +--
> > > > >blockdev.c | 85 
> > > > > +-
> > > > >tests/qemu-iotests/155 |  9 ++--
> > > > >tests/qemu-iotests/165 |  4 +-
> > > > >tests/qemu-iotests/245 | 27 +++-
> > > > >tests/qemu-iotests/248 |  2 +-
> > > > >tests/qemu-iotests/248.out |  2 +-
> > > > >tests/qemu-iotests/298 |  4 +-
> > > > >10 files changed, 89 insertions(+), 63 deletions(-)
> > > > > 
> > > > > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > > > > index c0e7c23331..b9fcf20a81 100644
> > > > > --- a/qapi/block-core.json
> > > > > +++ b/qapi/block-core.json
> > > > > @@ -4177,7 +4177,7 @@
> > > > ># Since: 4.0
> > > > >##
> > > > >{ 'command': 'x-blockdev-reopen',
> > > > > -  'data': 'BlockdevOptions', 'boxed': true }
> > > > > +  'data': { 'options': ['BlockdevOptions'] } }
> > > > 
> > > > Do we also want to drop x- prefix?
> > > 
> > > libvirt really wants to have a stable blockdev-reopen interface in 6.0
> > > because enabling the incremental backup code depends on this (they just
> > > toggle the readonly flag if I understand correctly, so most of the work
> > > we're currently doing isn't even relevant at this moment for libvirt).
> > 
> > Do you know what is the case exactly? If they do it to remove dirty bitmap
> > from backing image after snapshot operation, probably we'd better improve
> > block-dirty-bitmap-remove command to be able to reopen r-o image?
> > 
> > (I just recently faced such a task)
> 
> I think it was to switch nodes between read-only and read-write, but I
> don't remember the exact context.
> 
> Peter, can you add the details?

Libvirt indeed has code to drive blockdev-reopen to use it to toggle the
read-write state for block dirty bitmap manipulation.

Few of the cases where we use it:

- after blockjobs to toggle the destination image writable (the
  blockjobs turns it RO when it finishes) so that we can merge the
  appropriate bitmaps

- for deletion of bitmaps in backing images in the checkpoint APIs

Both of the operations really require blockdev-reopen and are required
for the incremental backup feature as whole since bitmap manipulation is
crucial for it and it doesn't happen automatically.

The blockdev code has the facilities to also use blockdev-reopen to
change RO/RW state for blockjobs if it will be needed in the future or
it can be later used to even replace the auto-read-only feature of the
protocol node drivers which were actually added to make -blockdev work
in libvirt's s-virt environment properly.




Re: [RFC PATCH v2 3/4] block: Support multiple reopening with x-blockdev-reopen

2021-03-01 Thread Vladimir Sementsov-Ogievskiy

01.03.2021 14:07, Kevin Wolf wrote:

Am 25.02.2021 um 18:02 hat Vladimir Sementsov-Ogievskiy geschrieben:

24.02.2021 15:33, Kevin Wolf wrote:

Am 09.02.2021 um 09:03 hat Vladimir Sementsov-Ogievskiy geschrieben:

08.02.2021 21:44, Alberto Garcia wrote:

Signed-off-by: Alberto Garcia 
---
qapi/block-core.json   |  2 +-
include/block/block.h  |  1 +
block.c| 16 +--
blockdev.c | 85 +-
tests/qemu-iotests/155 |  9 ++--
tests/qemu-iotests/165 |  4 +-
tests/qemu-iotests/245 | 27 +++-
tests/qemu-iotests/248 |  2 +-
tests/qemu-iotests/248.out |  2 +-
tests/qemu-iotests/298 |  4 +-
10 files changed, 89 insertions(+), 63 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index c0e7c23331..b9fcf20a81 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -4177,7 +4177,7 @@
# Since: 4.0
##
{ 'command': 'x-blockdev-reopen',
-  'data': 'BlockdevOptions', 'boxed': true }
+  'data': { 'options': ['BlockdevOptions'] } }


Do we also want to drop x- prefix?


libvirt really wants to have a stable blockdev-reopen interface in 6.0
because enabling the incremental backup code depends on this (they just
toggle the readonly flag if I understand correctly, so most of the work
we're currently doing isn't even relevant at this moment for libvirt).


Do you know what is the case exactly? If they do it to remove dirty bitmap
from backing image after snapshot operation, probably we'd better improve
block-dirty-bitmap-remove command to be able to reopen r-o image?

(I just recently faced such a task)


I think it was to switch nodes between read-only and read-write, but I
don't remember the exact context.



I already don't think that making implicit reopen-to-rw is a good idea. It's OK 
for blockdev-commit, but may be unexpected for bitmaps manipulation.




Given that the soft freeze is coming closer (March 16), I wonder if we
should just make this API change and declare the interface stable. We
can then make Vladimir's fixes and the file reopening on top of it - if
it's in time for 6.0, that would be good, but if not we could move it to
6.1 without impacting libvirt.

I think we're reasonable confident that the QAPI interfaces are right,
even if maybe not that all aspects of the implementation are right yet.

What do you think?


I think it's OK.. We have it since 4.0. What will we win keeping -x
for years? Even latest change from updating one device to several
could be easily done with help of 'alternate' if the command was
already stable.


I think your series is kind of important to really call the
implementation stable. We can always feature flags to indicate the fixes
if necessary, but it would still feel better to declare something stable
that doesn't have known bugs. :-)

Do you think your series will still take a while? Maybe my first
comments sounded a bit negative because it was really hard to review at
first without knowing the final state, but after all I think the
approach is sane and apart from some implementation details, we're not
that far away from getting it into a mergable state.



Thanks :)

I'm now busy with our bugs for Virtuozzo release.. Still, I hope, I'll have a 
chance to reroll permission-update series this week.

--
Best regards,
Vladimir



Re: [RFC PATCH v2 3/4] block: Support multiple reopening with x-blockdev-reopen

2021-03-01 Thread Kevin Wolf
Am 25.02.2021 um 18:02 hat Vladimir Sementsov-Ogievskiy geschrieben:
> 24.02.2021 15:33, Kevin Wolf wrote:
> > Am 09.02.2021 um 09:03 hat Vladimir Sementsov-Ogievskiy geschrieben:
> > > 08.02.2021 21:44, Alberto Garcia wrote:
> > > > Signed-off-by: Alberto Garcia 
> > > > ---
> > > >qapi/block-core.json   |  2 +-
> > > >include/block/block.h  |  1 +
> > > >block.c| 16 +--
> > > >blockdev.c | 85 
> > > > +-
> > > >tests/qemu-iotests/155 |  9 ++--
> > > >tests/qemu-iotests/165 |  4 +-
> > > >tests/qemu-iotests/245 | 27 +++-
> > > >tests/qemu-iotests/248 |  2 +-
> > > >tests/qemu-iotests/248.out |  2 +-
> > > >tests/qemu-iotests/298 |  4 +-
> > > >10 files changed, 89 insertions(+), 63 deletions(-)
> > > > 
> > > > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > > > index c0e7c23331..b9fcf20a81 100644
> > > > --- a/qapi/block-core.json
> > > > +++ b/qapi/block-core.json
> > > > @@ -4177,7 +4177,7 @@
> > > ># Since: 4.0
> > > >##
> > > >{ 'command': 'x-blockdev-reopen',
> > > > -  'data': 'BlockdevOptions', 'boxed': true }
> > > > +  'data': { 'options': ['BlockdevOptions'] } }
> > > 
> > > Do we also want to drop x- prefix?
> > 
> > libvirt really wants to have a stable blockdev-reopen interface in 6.0
> > because enabling the incremental backup code depends on this (they just
> > toggle the readonly flag if I understand correctly, so most of the work
> > we're currently doing isn't even relevant at this moment for libvirt).
> 
> Do you know what is the case exactly? If they do it to remove dirty bitmap
> from backing image after snapshot operation, probably we'd better improve
> block-dirty-bitmap-remove command to be able to reopen r-o image?
> 
> (I just recently faced such a task)

I think it was to switch nodes between read-only and read-write, but I
don't remember the exact context.

Peter, can you add the details?

> > Given that the soft freeze is coming closer (March 16), I wonder if we
> > should just make this API change and declare the interface stable. We
> > can then make Vladimir's fixes and the file reopening on top of it - if
> > it's in time for 6.0, that would be good, but if not we could move it to
> > 6.1 without impacting libvirt.
> > 
> > I think we're reasonable confident that the QAPI interfaces are right,
> > even if maybe not that all aspects of the implementation are right yet.
> > 
> > What do you think?
> 
> I think it's OK.. We have it since 4.0. What will we win keeping -x
> for years? Even latest change from updating one device to several
> could be easily done with help of 'alternate' if the command was
> already stable.

I think your series is kind of important to really call the
implementation stable. We can always feature flags to indicate the fixes
if necessary, but it would still feel better to declare something stable
that doesn't have known bugs. :-)

Do you think your series will still take a while? Maybe my first
comments sounded a bit negative because it was really hard to review at
first without knowing the final state, but after all I think the
approach is sane and apart from some implementation details, we're not
that far away from getting it into a mergable state.

Kevin




Re: [RFC PATCH v2 3/4] block: Support multiple reopening with x-blockdev-reopen

2021-02-26 Thread Alberto Garcia
On Wed 24 Feb 2021 01:33:05 PM CET, Kevin Wolf  wrote:
>> >   { 'command': 'x-blockdev-reopen',
>> > -  'data': 'BlockdevOptions', 'boxed': true }
>> > +  'data': { 'options': ['BlockdevOptions'] } }
>> 
>> Do we also want to drop x- prefix?
>
> libvirt really wants to have a stable blockdev-reopen interface in 6.0
> because enabling the incremental backup code depends on this (they
> just toggle the readonly flag if I understand correctly, so most of
> the work we're currently doing isn't even relevant at this moment for
> libvirt).
>
> Given that the soft freeze is coming closer (March 16), I wonder if we
> should just make this API change and declare the interface stable. We
> can then make Vladimir's fixes and the file reopening on top of it -
> if it's in time for 6.0, that would be good, but if not we could move
> it to 6.1 without impacting libvirt.

I expect to publish the new version of my patches next week, although
they still apply on top of Vladimir's code, which is not rebased.

We can of course simply update the API and implement the functionality
later, but apart from dropping the prefix we would also be changing the
parameters so qmp_x_blockdev_reopen() would also need to be modified.

Berto



Re: [RFC PATCH v2 3/4] block: Support multiple reopening with x-blockdev-reopen

2021-02-25 Thread Vladimir Sementsov-Ogievskiy

24.02.2021 15:33, Kevin Wolf wrote:

Am 09.02.2021 um 09:03 hat Vladimir Sementsov-Ogievskiy geschrieben:

08.02.2021 21:44, Alberto Garcia wrote:

Signed-off-by: Alberto Garcia 
---
   qapi/block-core.json   |  2 +-
   include/block/block.h  |  1 +
   block.c| 16 +--
   blockdev.c | 85 +-
   tests/qemu-iotests/155 |  9 ++--
   tests/qemu-iotests/165 |  4 +-
   tests/qemu-iotests/245 | 27 +++-
   tests/qemu-iotests/248 |  2 +-
   tests/qemu-iotests/248.out |  2 +-
   tests/qemu-iotests/298 |  4 +-
   10 files changed, 89 insertions(+), 63 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index c0e7c23331..b9fcf20a81 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -4177,7 +4177,7 @@
   # Since: 4.0
   ##
   { 'command': 'x-blockdev-reopen',
-  'data': 'BlockdevOptions', 'boxed': true }
+  'data': { 'options': ['BlockdevOptions'] } }


Do we also want to drop x- prefix?


libvirt really wants to have a stable blockdev-reopen interface in 6.0
because enabling the incremental backup code depends on this (they just
toggle the readonly flag if I understand correctly, so most of the work
we're currently doing isn't even relevant at this moment for libvirt).


Do you know what is the case exactly? If they do it to remove dirty bitmap
from backing image after snapshot operation, probably we'd better improve
block-dirty-bitmap-remove command to be able to reopen r-o image?

(I just recently faced such a task)



Given that the soft freeze is coming closer (March 16), I wonder if we
should just make this API change and declare the interface stable. We
can then make Vladimir's fixes and the file reopening on top of it - if
it's in time for 6.0, that would be good, but if not we could move it to
6.1 without impacting libvirt.

I think we're reasonable confident that the QAPI interfaces are right,
even if maybe not that all aspects of the implementation are right yet.

What do you think?



I think it's OK.. We have it since 4.0. What will we win keeping -x for years? 
Even latest change from updating one device to several could be easily done 
with help of 'alternate' if the command was already stable.


--
Best regards,
Vladimir



Re: [RFC PATCH v2 3/4] block: Support multiple reopening with x-blockdev-reopen

2021-02-24 Thread Kevin Wolf
Am 09.02.2021 um 09:03 hat Vladimir Sementsov-Ogievskiy geschrieben:
> 08.02.2021 21:44, Alberto Garcia wrote:
> > Signed-off-by: Alberto Garcia 
> > ---
> >   qapi/block-core.json   |  2 +-
> >   include/block/block.h  |  1 +
> >   block.c| 16 +--
> >   blockdev.c | 85 +-
> >   tests/qemu-iotests/155 |  9 ++--
> >   tests/qemu-iotests/165 |  4 +-
> >   tests/qemu-iotests/245 | 27 +++-
> >   tests/qemu-iotests/248 |  2 +-
> >   tests/qemu-iotests/248.out |  2 +-
> >   tests/qemu-iotests/298 |  4 +-
> >   10 files changed, 89 insertions(+), 63 deletions(-)
> > 
> > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > index c0e7c23331..b9fcf20a81 100644
> > --- a/qapi/block-core.json
> > +++ b/qapi/block-core.json
> > @@ -4177,7 +4177,7 @@
> >   # Since: 4.0
> >   ##
> >   { 'command': 'x-blockdev-reopen',
> > -  'data': 'BlockdevOptions', 'boxed': true }
> > +  'data': { 'options': ['BlockdevOptions'] } }
> 
> Do we also want to drop x- prefix?

libvirt really wants to have a stable blockdev-reopen interface in 6.0
because enabling the incremental backup code depends on this (they just
toggle the readonly flag if I understand correctly, so most of the work
we're currently doing isn't even relevant at this moment for libvirt).

Given that the soft freeze is coming closer (March 16), I wonder if we
should just make this API change and declare the interface stable. We
can then make Vladimir's fixes and the file reopening on top of it - if
it's in time for 6.0, that would be good, but if not we could move it to
6.1 without impacting libvirt.

I think we're reasonable confident that the QAPI interfaces are right,
even if maybe not that all aspects of the implementation are right yet.

What do you think?

Kevin




Re: [RFC PATCH v2 3/4] block: Support multiple reopening with x-blockdev-reopen

2021-02-16 Thread Alberto Garcia
On Tue 09 Feb 2021 09:03:02 AM CET, Vladimir Sementsov-Ogievskiy wrote:
>>   { 'command': 'x-blockdev-reopen',
>> -  'data': 'BlockdevOptions', 'boxed': true }
>> +  'data': { 'options': ['BlockdevOptions'] } }
>
> Do we also want to drop x- prefix?

I think we can drop it once it's clear the the API is fine. It can be on
a separate patch after this.

>> -visit_free(v);
>> +bdrv_reopen_queue_free(queue);
>> +g_slist_free_full(drained, (GDestroyNotify) bdrv_subtree_drained_end);
>> +g_slist_free_full(aio_ctxs, (GDestroyNotify) aio_context_release);
>> +g_slist_free_full(visitors, (GDestroyNotify) visit_free);
>
> Probably you can use g_autoslist() for defining these lists to get
> automatic cleanup.

g_autoslist() requires that the type has a cleanup function, but that's
not the case here and I don't think we can add one ('drained' contains a
BlockDriverState, what's the cleanup function? bdrv_subtree_drained_end
or bdrv_unref?)

I think it's fine to call g_slist_free_full() explicitly in this case.

Berto



Re: [RFC PATCH v2 3/4] block: Support multiple reopening with x-blockdev-reopen

2021-02-09 Thread Vladimir Sementsov-Ogievskiy

08.02.2021 21:44, Alberto Garcia wrote:

Signed-off-by: Alberto Garcia 
---
  qapi/block-core.json   |  2 +-
  include/block/block.h  |  1 +
  block.c| 16 +--
  blockdev.c | 85 +-
  tests/qemu-iotests/155 |  9 ++--
  tests/qemu-iotests/165 |  4 +-
  tests/qemu-iotests/245 | 27 +++-
  tests/qemu-iotests/248 |  2 +-
  tests/qemu-iotests/248.out |  2 +-
  tests/qemu-iotests/298 |  4 +-
  10 files changed, 89 insertions(+), 63 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index c0e7c23331..b9fcf20a81 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -4177,7 +4177,7 @@
  # Since: 4.0
  ##
  { 'command': 'x-blockdev-reopen',
-  'data': 'BlockdevOptions', 'boxed': true }
+  'data': { 'options': ['BlockdevOptions'] } }


Do we also want to drop x- prefix?

  
  ##

  # @blockdev-del:
diff --git a/include/block/block.h b/include/block/block.h
index 6dd687a69e..fe4a220da9 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -372,6 +372,7 @@ BlockDriverState *bdrv_new_open_driver(BlockDriver *drv, 
const char *node_name,
  BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
  BlockDriverState *bs, QDict *options,
  bool keep_old_opts);
+void bdrv_reopen_queue_free(BlockReopenQueue *bs_queue);
  int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp);
  int bdrv_reopen_set_read_only(BlockDriverState *bs, bool read_only,
Error **errp);
diff --git a/block.c b/block.c
index 19b62da4af..b4fef2308f 100644
--- a/block.c
+++ b/block.c
@@ -3933,6 +3933,17 @@ BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue 
*bs_queue,
 NULL, 0, keep_old_opts);
  }
  
+void bdrv_reopen_queue_free(BlockReopenQueue *bs_queue)

+{
+if (bs_queue) {
+BlockReopenQueueEntry *bs_entry, *next;
+QTAILQ_FOREACH_SAFE(bs_entry, bs_queue, entry, next) {
+g_free(bs_entry);
+}
+g_free(bs_queue);
+}
+}
+
  /*
   * Reopen multiple BlockDriverStates atomically & transactionally.
   *
@@ -4024,10 +4035,7 @@ abort:
  }
  
  cleanup:

-QTAILQ_FOREACH_SAFE(bs_entry, bs_queue, entry, next) {
-g_free(bs_entry);
-}
-g_free(bs_queue);
+bdrv_reopen_queue_free(bs_queue);


this may be a separate patch

  
  return ret;

  }
diff --git a/blockdev.c b/blockdev.c
index 098a05709d..6b688c0f73 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3528,38 +3528,16 @@ fail:
  visit_free(v);
  }
  
-void qmp_x_blockdev_reopen(BlockdevOptions *options, Error **errp)

+void qmp_x_blockdev_reopen(BlockdevOptionsList *reopen_list, Error **errp)
  {
-BlockDriverState *bs;
-QObject *obj;
-Visitor *v = qobject_output_visitor_new();
-BlockReopenQueue *queue;
-QDict *qdict;
-
-/* Check for the selected node name */
-if (!options->has_node_name) {
-error_setg(errp, "Node name not specified");
-goto fail;
-}
-
-bs = bdrv_find_node(options->node_name);
-if (!bs) {
-error_setg(errp, "Cannot find node named '%s'", options->node_name);
-goto fail;
-}
-
-/* Put all options in a QDict and flatten it */
-visit_type_BlockdevOptions(v, NULL, , _abort);
-visit_complete(v, );
-qdict = qobject_to(QDict, obj);
-
-qdict_flatten(qdict);
-
-/* Perform the reopen operation */
+BlockReopenQueue *queue = NULL;
+GSList *aio_ctxs = NULL;
+GSList *visitors = NULL;
+GSList *drained = NULL;
  BdrvNextIterator it;
-GSList *aio_ctxs = NULL, *ctx;
  BlockDriverState *it_bs;
  
+/* Acquire all AIO contexts */

  for (it_bs = bdrv_first(); it_bs; it_bs = bdrv_next()) {
  AioContext *aio_context = bdrv_get_aio_context(it_bs);
  
@@ -3569,19 +3547,50 @@ void qmp_x_blockdev_reopen(BlockdevOptions *options, Error **errp)

  }
  }
  
-bdrv_subtree_drained_begin(bs);

-queue = bdrv_reopen_queue(NULL, bs, qdict, false);
+/* Add each one of the BDS that we want to reopen to the queue */
+for (; reopen_list != NULL; reopen_list = reopen_list->next) {
+BlockdevOptions *options = reopen_list->value;
+QDict *qdict;
+Visitor *v;
+BlockDriverState *bs;
+QObject *obj;
+
+/* Check for the selected node name */
+if (!options->has_node_name) {
+error_setg(errp, "Node name not specified");
+goto fail;
+}
+
+bs = bdrv_find_node(options->node_name);
+if (!bs) {
+error_setg(errp, "Cannot find node named '%s'", 
options->node_name);
+goto fail;
+}
+
+v = qobject_output_visitor_new();
+visitors = g_slist_prepend(visitors, v);
+
+/* Put all options in a QDict and flatten it */
+

[RFC PATCH v2 3/4] block: Support multiple reopening with x-blockdev-reopen

2021-02-08 Thread Alberto Garcia
Signed-off-by: Alberto Garcia 
---
 qapi/block-core.json   |  2 +-
 include/block/block.h  |  1 +
 block.c| 16 +--
 blockdev.c | 85 +-
 tests/qemu-iotests/155 |  9 ++--
 tests/qemu-iotests/165 |  4 +-
 tests/qemu-iotests/245 | 27 +++-
 tests/qemu-iotests/248 |  2 +-
 tests/qemu-iotests/248.out |  2 +-
 tests/qemu-iotests/298 |  4 +-
 10 files changed, 89 insertions(+), 63 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index c0e7c23331..b9fcf20a81 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -4177,7 +4177,7 @@
 # Since: 4.0
 ##
 { 'command': 'x-blockdev-reopen',
-  'data': 'BlockdevOptions', 'boxed': true }
+  'data': { 'options': ['BlockdevOptions'] } }
 
 ##
 # @blockdev-del:
diff --git a/include/block/block.h b/include/block/block.h
index 6dd687a69e..fe4a220da9 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -372,6 +372,7 @@ BlockDriverState *bdrv_new_open_driver(BlockDriver *drv, 
const char *node_name,
 BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
 BlockDriverState *bs, QDict *options,
 bool keep_old_opts);
+void bdrv_reopen_queue_free(BlockReopenQueue *bs_queue);
 int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp);
 int bdrv_reopen_set_read_only(BlockDriverState *bs, bool read_only,
   Error **errp);
diff --git a/block.c b/block.c
index 19b62da4af..b4fef2308f 100644
--- a/block.c
+++ b/block.c
@@ -3933,6 +3933,17 @@ BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue 
*bs_queue,
NULL, 0, keep_old_opts);
 }
 
+void bdrv_reopen_queue_free(BlockReopenQueue *bs_queue)
+{
+if (bs_queue) {
+BlockReopenQueueEntry *bs_entry, *next;
+QTAILQ_FOREACH_SAFE(bs_entry, bs_queue, entry, next) {
+g_free(bs_entry);
+}
+g_free(bs_queue);
+}
+}
+
 /*
  * Reopen multiple BlockDriverStates atomically & transactionally.
  *
@@ -4024,10 +4035,7 @@ abort:
 }
 
 cleanup:
-QTAILQ_FOREACH_SAFE(bs_entry, bs_queue, entry, next) {
-g_free(bs_entry);
-}
-g_free(bs_queue);
+bdrv_reopen_queue_free(bs_queue);
 
 return ret;
 }
diff --git a/blockdev.c b/blockdev.c
index 098a05709d..6b688c0f73 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3528,38 +3528,16 @@ fail:
 visit_free(v);
 }
 
-void qmp_x_blockdev_reopen(BlockdevOptions *options, Error **errp)
+void qmp_x_blockdev_reopen(BlockdevOptionsList *reopen_list, Error **errp)
 {
-BlockDriverState *bs;
-QObject *obj;
-Visitor *v = qobject_output_visitor_new();
-BlockReopenQueue *queue;
-QDict *qdict;
-
-/* Check for the selected node name */
-if (!options->has_node_name) {
-error_setg(errp, "Node name not specified");
-goto fail;
-}
-
-bs = bdrv_find_node(options->node_name);
-if (!bs) {
-error_setg(errp, "Cannot find node named '%s'", options->node_name);
-goto fail;
-}
-
-/* Put all options in a QDict and flatten it */
-visit_type_BlockdevOptions(v, NULL, , _abort);
-visit_complete(v, );
-qdict = qobject_to(QDict, obj);
-
-qdict_flatten(qdict);
-
-/* Perform the reopen operation */
+BlockReopenQueue *queue = NULL;
+GSList *aio_ctxs = NULL;
+GSList *visitors = NULL;
+GSList *drained = NULL;
 BdrvNextIterator it;
-GSList *aio_ctxs = NULL, *ctx;
 BlockDriverState *it_bs;
 
+/* Acquire all AIO contexts */
 for (it_bs = bdrv_first(); it_bs; it_bs = bdrv_next()) {
 AioContext *aio_context = bdrv_get_aio_context(it_bs);
 
@@ -3569,19 +3547,50 @@ void qmp_x_blockdev_reopen(BlockdevOptions *options, 
Error **errp)
 }
 }
 
-bdrv_subtree_drained_begin(bs);
-queue = bdrv_reopen_queue(NULL, bs, qdict, false);
+/* Add each one of the BDS that we want to reopen to the queue */
+for (; reopen_list != NULL; reopen_list = reopen_list->next) {
+BlockdevOptions *options = reopen_list->value;
+QDict *qdict;
+Visitor *v;
+BlockDriverState *bs;
+QObject *obj;
+
+/* Check for the selected node name */
+if (!options->has_node_name) {
+error_setg(errp, "Node name not specified");
+goto fail;
+}
+
+bs = bdrv_find_node(options->node_name);
+if (!bs) {
+error_setg(errp, "Cannot find node named '%s'", 
options->node_name);
+goto fail;
+}
+
+v = qobject_output_visitor_new();
+visitors = g_slist_prepend(visitors, v);
+
+/* Put all options in a QDict and flatten it */
+visit_type_BlockdevOptions(v, NULL, , _abort);
+visit_complete(v, );
+qdict = qobject_to(QDict, obj);
+
+qdict_flatten(qdict);
+
+bdrv_subtree_drained_begin(bs);
+