On Wed, Mar 01, 2017 at 11:53:14AM +0800, Hailiang Zhang wrote:
> On 2017/2/28 1:10, Stefan Hajnoczi wrote:
> > On Fri, Jan 20, 2017 at 11:47:56AM +0800, zhanghailiang wrote:
> > > @@ -119,12 +136,31 @@ static int replication_open(BlockDriverState *bs,
> > > QDict *options,
> > > "The option mode's value should be primary or
> > > secondary");
> > > goto fail;
> > > }
> > > + s->is_shared_disk = qemu_opt_get_bool(opts, REPLICATION_SHARED_DISK,
> > > + false);
> > > + if (s->is_shared_disk && (s->mode == REPLICATION_MODE_PRIMARY)) {
> > > + shared_disk_id = qemu_opt_get(opts, REPLICATION_SHARED_DISK_ID);
> > > + if (!shared_disk_id) {
> > > + error_setg(&local_err, "Missing shared disk blk option");
> > > + goto fail;
> > > + }
> > > + s->shared_disk_id = g_strdup(shared_disk_id);
> > > + blk = blk_by_name(s->shared_disk_id);
> > > + if (!blk) {
> > > + error_setg(&local_err, "There is no %s block",
> > > s->shared_disk_id);
> > > + goto fail;
> > > + }
> > > + /* We can't access root member of BlockBackend directly */
> > > + tmp_bs = blk_bs(blk);
> > > + s->primary_disk = QLIST_FIRST(&tmp_bs->parents);
> >
> > Why is this necessary?
> >
> > We already have the BlockBackend for the primary disk. I'm not sure why
> > the BdrvChild is needed.
> >
>
> Er, the helper backup_job_create() needs the BlockDriverState for the primary
> disk,You can get the BDS any time using blk_bs(). > besides, we want to make it same with 'secondary_disk' in struct > BDRVReplicationState. Block drivers are allowed to access BDS->parents so it's not necessarily a problem that the code does this. But it's rare and usually means there is a non-standard relationship between BDS nodes. It wasn't obvious from the code why you are going through the trouble of getting a BdrvChild instead of using the BlockBackend (the normal way of accessing a drive). Perhaps a comment would help: /* We have a BlockBackend for the primary disk but use BdrvChild for * consistency - active_disk, secondary_disk, etc are also BdrvChild. */ Stefan
signature.asc
Description: PGP signature
