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

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

2016-01-29 Thread Max Reitz

On 28.01.2016 04:33, Fam Zheng wrote:
> On Wed, 01/27 18:59, Max Reitz wrote:
>> Signed-off-by: Max Reitz 
>> ---
>>  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, _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(_bdrv_states, bs, monitor_list);
>>  }
>>  
>>  if (bs && bdrv_key_required(bs)) {
>>  if (blk) {
>>  blk_unref(blk);
>>  } else {
>> +QTAILQ_REMOVE(_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(_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 000..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
>>




signature.asc
Description: OpenPGP digital signature

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

2016-01-29 Thread Kevin Wolf

Am 29.01.2016 um 14:44 hat Max Reitz geschrieben:
> On 28.01.2016 04:33, Fam Zheng wrote:
> > On Wed, 01/27 18:59, Max Reitz wrote:
> >> Signed-off-by: Max Reitz 
> >> ---
> >>  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, _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(_bdrv_states, bs, monitor_list);
> >>  }
> >>  
> >>  if (bs && bdrv_key_required(bs)) {
> >>  if (blk) {
> >>  blk_unref(blk);
> >>  } else {
> >> +QTAILQ_REMOVE(_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.

Actually, if you were to respin the series, you could remove the check
that it replaces:

if (bs->refcnt > 1 || !QLIST_EMPTY(>parents)) {

The QLIST_EMPTY() check is trying to achieve the same as what you
introduce in a clean way now. It doesn't hurt at the moment, but I had
to get rid of the QLIST_EMPTY() check when I tried to convert BB to
BdrvChild.

Kevin


pgpXpebeXQaKZ.pgp
Description: PGP signature

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

2016-01-27 Thread Fam Zheng

On Wed, 01/27 18:59, Max Reitz wrote:
> Signed-off-by: Max Reitz 
> ---
>  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, _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(_bdrv_states, bs, monitor_list);
>  }
>  
>  if (bs && bdrv_key_required(bs)) {
>  if (blk) {
>  blk_unref(blk);
>  } else {
> +QTAILQ_REMOVE(_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? Deserve some words in the
commit message?

>  }
>  
>  if (blk) {
>  blk_unref(blk);
>  } else {
> +QTAILQ_REMOVE(_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 000..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
>

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

2016-01-27 Thread Max Reitz

Signed-off-by: Max Reitz 
---
 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, _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(_bdrv_states, bs, monitor_list);
 }
 
 if (bs && bdrv_key_required(bs)) {
 if (blk) {
 blk_unref(blk);
 } else {
+QTAILQ_REMOVE(_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;
+}
 }
 
 if (blk) {
 blk_unref(blk);
 } else {
+QTAILQ_REMOVE(_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 000..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

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

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

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

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

4 matches

Site Navigation

Mail list logo

Footer information