| Message ID | 1453917600-2663-13-git-send-email-mreitz@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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? Deserve some words in the commit message? > } > > 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 >
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 >>
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 <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. Actually, if you were to respin the series, you could remove the check that it replaces: if (bs->refcnt > 1 || !QLIST_EMPTY(&bs->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
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; + } } 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) +{ +}
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