Re: [Qemu-devel] [PATCH v8 12/16] blockdev: Keep track of monitor-owned BDS

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 4689 bytes --]

On 28.01.2016 04:33, Fam Zheng wrote:
> On Wed, 01/27 18:59, Max Reitz wrote:
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>  blockdev.c                             | 26 ++++++++++++++++++++++++++
>>  include/block/block_int.h              |  4 ++++
>>  stubs/Makefile.objs                    |  1 +
>>  stubs/blockdev-close-all-bdrv-states.c |  5 +++++
>>  4 files changed, 36 insertions(+)
>>  create mode 100644 stubs/blockdev-close-all-bdrv-states.c
>>
>> diff --git a/blockdev.c b/blockdev.c
>> index 09d4621..ac93f43 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
>> @@ -50,6 +50,9 @@
>>  #include "trace.h"
>>  #include "sysemu/arch_init.h"
>>  
>> +static QTAILQ_HEAD(, BlockDriverState) monitor_bdrv_states =
>> +    QTAILQ_HEAD_INITIALIZER(monitor_bdrv_states);
>> +
>>  static const char *const if_name[IF_COUNT] = {
>>      [IF_NONE] = "none",
>>      [IF_IDE] = "ide",
>> @@ -702,6 +705,19 @@ fail:
>>      return NULL;
>>  }
>>  
>> +void blockdev_close_all_bdrv_states(void)
>> +{
>> +    BlockDriverState *bs, *next_bs;
>> +
>> +    QTAILQ_FOREACH_SAFE(bs, &monitor_bdrv_states, monitor_list, next_bs) {
>> +        AioContext *ctx = bdrv_get_aio_context(bs);
>> +
>> +        aio_context_acquire(ctx);
>> +        bdrv_unref(bs);
>> +        aio_context_release(ctx);
>> +    }
>> +}
>> +
>>  static void qemu_opt_rename(QemuOpts *opts, const char *from, const char *to,
>>                              Error **errp)
>>  {
>> @@ -3875,12 +3891,15 @@ void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
>>          if (!bs) {
>>              goto fail;
>>          }
>> +
>> +        QTAILQ_INSERT_TAIL(&monitor_bdrv_states, bs, monitor_list);
>>      }
>>  
>>      if (bs && bdrv_key_required(bs)) {
>>          if (blk) {
>>              blk_unref(blk);
>>          } else {
>> +            QTAILQ_REMOVE(&monitor_bdrv_states, bs, monitor_list);
>>              bdrv_unref(bs);
>>          }
>>          error_setg(errp, "blockdev-add doesn't support encrypted devices");
>> @@ -3945,11 +3964,18 @@ void qmp_x_blockdev_del(bool has_id, const char *id,
>>                         bdrv_get_device_or_node_name(bs));
>>              goto out;
>>          }
>> +
>> +        if (!blk && !bs->monitor_list.tqe_prev) {
>> +            error_setg(errp, "Node %s is not owned by the monitor",
>> +                       bs->node_name);
>> +            goto out;
>> +        }
> 
> Is this an extra restriction added by this patch?

I hope not. This is just an additional check that should not change
behavior; if it does, we did something wrong.

>                                                   Deserve some words in the
> commit message?

I'll see if I can come up with something.

Max

>>      }
>>  
>>      if (blk) {
>>          blk_unref(blk);
>>      } else {
>> +        QTAILQ_REMOVE(&monitor_bdrv_states, bs, monitor_list);
>>          bdrv_unref(bs);
>>      }
>>  
>> diff --git a/include/block/block_int.h b/include/block/block_int.h
>> index 1e4c518..dd00d12 100644
>> --- a/include/block/block_int.h
>> +++ b/include/block/block_int.h
>> @@ -445,6 +445,8 @@ struct BlockDriverState {
>>      QTAILQ_ENTRY(BlockDriverState) device_list;
>>      /* element of the list of all BlockDriverStates (all_bdrv_states) */
>>      QTAILQ_ENTRY(BlockDriverState) bs_list;
>> +    /* element of the list of monitor-owned BDS */
>> +    QTAILQ_ENTRY(BlockDriverState) monitor_list;
>>      QLIST_HEAD(, BdrvDirtyBitmap) dirty_bitmaps;
>>      int refcnt;
>>  
>> @@ -707,4 +709,6 @@ bool bdrv_requests_pending(BlockDriverState *bs);
>>  void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap **out);
>>  void bdrv_undo_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap *in);
>>  
>> +void blockdev_close_all_bdrv_states(void);
>> +
>>  #endif /* BLOCK_INT_H */
>> diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
>> index d7898a0..e922de9 100644
>> --- a/stubs/Makefile.objs
>> +++ b/stubs/Makefile.objs
>> @@ -1,5 +1,6 @@
>>  stub-obj-y += arch-query-cpu-def.o
>>  stub-obj-y += bdrv-commit-all.o
>> +stub-obj-y += blockdev-close-all-bdrv-states.o
>>  stub-obj-y += clock-warp.o
>>  stub-obj-y += cpu-get-clock.o
>>  stub-obj-y += cpu-get-icount.o
>> diff --git a/stubs/blockdev-close-all-bdrv-states.c b/stubs/blockdev-close-all-bdrv-states.c
>> new file mode 100644
>> index 0000000..12d2442
>> --- /dev/null
>> +++ b/stubs/blockdev-close-all-bdrv-states.c
>> @@ -0,0 +1,5 @@
>> +#include "block/block_int.h"
>> +
>> +void blockdev_close_all_bdrv_states(void)
>> +{
>> +}
>> -- 
>> 2.7.0
>>



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]