On Fri, Feb 05, 2016 at 12:18:07PM +0800, Changlong Xie wrote:
+static void replication_start(ReplicationState *rs, ReplicationMode mode,
+ Error **errp)
+{
+ BlockDriverState *bs = rs->opaque;
+ BDRVReplicationState *s = bs->opaque;
+ int64_t active_length, hidden_length, disk_length;
+ AioContext *aio_context;
+ Error *local_err = NULL;
+
+ if (s->replication_state != BLOCK_REPLICATION_NONE) {
Dereferencing bs and s is not thread-safe since the AioContext isn't
acquired here. Unless you have other locking rules, the default is that
everything in BlockDriverState and bs->opaque must be accessed with the
AioContext acquired.
Please apply this comment to the rest of the patch series, too.
Functions that are not part of BlockDriver are probably called outside
the AioContext and must acquire it before they are allowed to access
anything else in bs.
+ s->top_bs = get_top_bs(bs);
+ if (!s->top_bs) {
+ error_setg(errp, "Cannot get the top block driver state to do"
+ " internal backup");
+ return;
+ }
Traversing the BDS graph using the parent pointer is not safe - you
cannot stash the parent pointer because it changes if a parent node is
inserted/removed.
I suggest passing the drive name as an argument so that bdrv_lookup_bs()
can be used when installing the op blocker. Then the BdrvChild.parent
pointer patch and get_top_bs() can be deleted.