On 03/11/2015 09:58 AM, Stefan Hajnoczi wrote:
On Mon, Mar 02, 2015 at 06:19:50PM -0500, John Snow wrote:+static BdrvDirtyBitmap *block_dirty_bitmap_lookup(const char *node, + const char *name, + BlockDriverState **pbs, + Error **errp) +{ + BlockDriverState *bs; + BdrvDirtyBitmap *bitmap; + + if (!node) { + error_setg(errp, "Node cannot be NULL"); + return NULL; + } + if (!name) { + error_setg(errp, "Bitmap name cannot be NULL"); + return NULL; + } + + bs = bdrv_lookup_bs(node, node, NULL); + if (!bs) { + error_setg(errp, "Node '%s' not found", node); + return NULL; + } + + /* If caller provided a BDS*, provide the result of that lookup, too. */ + if (pbs) { + *pbs = bs; + } + + bitmap = bdrv_find_dirty_bitmap(bs, name);AioContext is not held here. I'm worried that a block job (running under the AioContext) could change or delete the bitmap at the same time as this function is running. This function should acquire the AioContext before calling bdrv_find_dirty_bitmap() and fill out an AioContext** parameter so the caller can continue to use bs/bitmap under the lock.
OK, I'll audit all uses of this function with this in mind.
