On 11/06/2015 06:18 PM, Stefan Hajnoczi wrote:
On Wed, Nov 04, 2015 at 08:19:39PM +0300, Denis V. Lunev wrote:
+BlockDriverState *bdrv_all_find_vmstate_bs(void)
+{
+ BlockDriverState *bs = NULL;
+
+ while ((bs = bdrv_next(bs))) {
+ AioContext *ctx = bdrv_get_aio_context(bs);
+
+ aio_context_acquire(ctx);
+ if (bdrv_can_snapshot(bs)) {
+ return bs;
This leaves AioContext acquired. If that is intentional then it must be
documented because it looks like a bug.
Normally functions that do this have an AioContext **aio_context agument
so the caller does aio_context_release() later. This way it's obvious
that the caller needs to release.
For example, see blockdev.c:find_block_job().
+ }
+ aio_context_release(ctx);
+ }
+ return NULL;
+}
+
+void bdrv_unlock(BlockDriverState *bs)
+{
+ aio_context_release(bdrv_get_aio_context(bs));
+}
This API is weird. There is no lock function. Please do what I
mentioned above.
Another advantage of that approach is that we are 100% sure to release
the same AioContext that was acquired (even if bdrv_set_aio_context()
gets called halfway through).
no prob if Juan will accept that :) Ho does not want to care
and take the lock anywhere in his code. For me this is
pure matter of taste.
