From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48497) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yb2GG-0005iN-Bl for qemu-devel@nongnu.org; Thu, 26 Mar 2015 03:31:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Yb2GD-0002Gy-4X for qemu-devel@nongnu.org; Thu, 26 Mar 2015 03:31:28 -0400 Date: Thu, 26 Mar 2015 15:31:15 +0800 From: Fam Zheng Message-ID: <20150326073115.GK14724@ad.nay.redhat.com> References: <1427276174-9130-1-git-send-email-wency@cn.fujitsu.com> <1427276174-9130-10-git-send-email-wency@cn.fujitsu.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable In-Reply-To: <1427276174-9130-10-git-send-email-wency@cn.fujitsu.com> Subject: Re: [Qemu-devel] [RFC PATCH COLO v2 09/13] block: Parse "backing_reference" option to reference existing BDS List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Wen Congyang Cc: Kevin Wolf , Yang Hongyang , Lai Jiangshan , qemu block , Jiang Yunhong , Dong Eddie , "Dr. David Alan Gilbert" , qemu devel , Gonglei , Stefan Hajnoczi , Paolo Bonzini , Max Reitz , zhanghailiang On Wed, 03/25 17:36, Wen Congyang wrote: > Usage: > -drive file=3Dxxx,id=3DY, \ > -drive file=3Dxxxx,id=3DX,backing_reference.drive_id=3DY,backing_referenc= e.hidden-disk.* >=20 > It will create such backing chain: > {virtio-blk dev 'Y'} = {virtio-blk dev 'X'} > | = | > | = | > v = v >=20 > [base] <- [mid] <- ( Y ) <----------------- (hidden target) <-------= -------- ( X ) >=20 > v ^ > v ^ > v ^ > v ^ > >>>> drive-backup sync=3Dnone >>>> >=20 > X's backing file is hidden-disk, and hidden-disk's backing file is Y. > Disk Y may be opened or reopened in read-write mode, so A block backup > job is automatically created: source is Y and target is hidden-disk. >=20 > Signed-off-by: Wen Congyang > Signed-off-by: zhanghailiang > Signed-off-by: Gonglei > --- > block.c | 145 +++++++++++++++++++++++++++++++++++++++= +++++- > include/block/block.h | 1 + > include/block/block_int.h | 1 + > tests/qemu-iotests/051 | 13 ++++ > tests/qemu-iotests/051.out | 13 ++++ > 5 files changed, 170 insertions(+), 3 deletions(-) >=20 > diff --git a/block.c b/block.c > index b4d629e..bd7fa9c 100644 > --- a/block.c > +++ b/block.c > @@ -1351,6 +1351,113 @@ free_exit: > return ret; > } > =20 > +static void backing_reference_completed(void *opaque, int ret) > +{ > + BlockDriverState *hidden_disk =3D opaque; > + > + assert(!hidden_disk->backing_reference); > +} > + > +static int bdrv_open_backing_reference_file(BlockDriverState *bs, > + QDict *options, Error **errp) > +{ > + const char *backing_name; > + QDict *hidden_disk_options =3D NULL; > + BlockDriverState *backing_hd, *hidden_disk; > + BlockBackend *backing_blk; > + Error *local_err =3D NULL; > + int ret =3D 0; > + > + backing_name =3D qdict_get_try_str(options, "drive_id"); > + if (!backing_name) { > + error_setg(errp, "Backing reference needs option drive_id"); > + ret =3D -EINVAL; > + goto free_exit; > + } > + qdict_del(options, "drive_id"); > + > + qdict_extract_subqdict(options, &hidden_disk_options, "hidden-disk."= ); > + if (!qdict_size(hidden_disk_options)) { > + error_setg(errp, "Backing reference needs option hidden-disk.*"); > + ret =3D -EINVAL; > + goto free_exit; > + } > + > + if (qdict_size(options)) { > + const QDictEntry *entry =3D qdict_first(options); > + error_setg(errp, "Backing reference used by '%s' doesn't support= " > + "the option '%s'", bdrv_get_device_name(bs), entry->k= ey); > + ret =3D -EINVAL; > + goto free_exit; > + } > + > + backing_blk =3D blk_by_name(backing_name); > + if (!backing_blk) { > + error_set(errp, QERR_DEVICE_NOT_FOUND, backing_name); > + ret =3D -ENOENT; > + goto free_exit; > + } > + > + backing_hd =3D blk_bs(backing_blk); > + /* Backing reference itself? */ > + if (backing_hd =3D=3D bs || bdrv_find_overlay(backing_hd, bs)) { > + error_setg(errp, "Backing reference itself"); > + ret =3D -EINVAL; > + goto free_exit; > + } > + > + if (bdrv_op_is_blocked(backing_hd, BLOCK_OP_TYPE_BACKING_REFERENCE, > + errp)) { > + ret =3D -EBUSY; > + goto free_exit; > + } > + > + /* hidden-disk is bs's backing file */ > + ret =3D bdrv_open_backing_file(bs, hidden_disk_options, errp); > + hidden_disk_options =3D NULL; > + if (ret < 0) { > + goto free_exit; > + } > + > + hidden_disk =3D bs->backing_hd; > + if (!hidden_disk->drv || !hidden_disk->drv->supports_backing) { > + ret =3D -EINVAL; > + error_setg(errp, "Hidden disk's driver doesn't support backing f= iles"); > + goto free_exit; > + } > + > + bdrv_set_backing_hd(hidden_disk, backing_hd); > + bdrv_ref(backing_hd); > + > + /* > + * backing hd may be opened or reopened in read-write mode, so we > + * should backup backing hd to hidden disk > + */ > + bdrv_op_unblock(hidden_disk, BLOCK_OP_TYPE_BACKUP_TARGET, > + bs->backing_blocker); > + bdrv_op_unblock(backing_hd, BLOCK_OP_TYPE_BACKUP_SOURCE, > + hidden_disk->backing_blocker); > + > + bdrv_ref(hidden_disk); > + backup_start(backing_hd, hidden_disk, 0, MIRROR_SYNC_MODE_NONE, > + BLOCKDEV_ON_ERROR_REPORT, BLOCKDEV_ON_ERROR_REPORT, > + backing_reference_completed, hidden_disk, &local_err); We need to be careful, the backing_hd may not be safe to access, depending = on which AioContext it is running on. At least, we should make sure bs, hidden_disk and backing_hd are all on the same AioContext. Here, before accessing backing_hd, we need to call aio_context_acquire, lik= e in qmp_drive_backup. Fam > + if (local_err) { > + error_propagate(errp, local_err); > + bdrv_unref(hidden_disk); > + /* FIXME, use which errno? */ > + ret =3D -EIO; > + goto free_exit; > + } > + > + bs->backing_reference =3D true; > + > +free_exit: > + QDECREF(hidden_disk_options); > + QDECREF(options); > + return ret; > +} > + > /* > * Opens a disk image whose options are given as BlockdevRef in another = block > * device's options. > @@ -1604,13 +1711,37 @@ int bdrv_open(BlockDriverState **pbs, const char = *filename, > =20 > /* If there is a backing file, use it */ > if ((flags & BDRV_O_NO_BACKING) =3D=3D 0) { > - QDict *backing_options; > + QDict *backing_options, *backing_reference_options; > =20 > + qdict_extract_subqdict(options, &backing_reference_options, > + "backing_reference."); > qdict_extract_subqdict(options, &backing_options, "backing."); > - ret =3D bdrv_open_backing_file(bs, backing_options, &local_err); > - if (ret < 0) { > + > + if (qdict_size(backing_reference_options) && > + qdict_size(backing_options)) { > + error_setg(&local_err, > + "Option \"backing_reference.*\" and \"backing.*\"" > + " cannot be used together"); > + ret =3D -EINVAL; > + QDECREF(backing_reference_options); > + QDECREF(backing_options); > goto close_and_fail; > } > + if (qdict_size(backing_reference_options)) { > + QDECREF(backing_options); > + ret =3D bdrv_open_backing_reference_file(bs, > + backing_reference_opt= ions, > + &local_err); > + if (ret) { > + goto close_and_fail; > + } > + } else { > + QDECREF(backing_reference_options); > + ret =3D bdrv_open_backing_file(bs, backing_options, &local_e= rr); > + if (ret < 0) { > + goto close_and_fail; > + } > + } > } > =20 > bdrv_refresh_filename(bs); > @@ -1941,6 +2072,14 @@ void bdrv_close(BlockDriverState *bs) > if (bs->drv) { > if (bs->backing_hd) { > BlockDriverState *backing_hd =3D bs->backing_hd; > + if (bs->backing_reference) { > + assert(backing_hd->backing_hd); > + if (backing_hd->backing_hd->job) { > + block_job_cancel(backing_hd->backing_hd->job); > + } > + bdrv_set_backing_hd(backing_hd, NULL); > + bdrv_unref(backing_hd->backing_hd); > + } > bdrv_set_backing_hd(bs, NULL); > bdrv_unref(backing_hd); > } > diff --git a/include/block/block.h b/include/block/block.h > index 68f3b1a..7138e90 100644 > --- a/include/block/block.h > +++ b/include/block/block.h > @@ -159,6 +159,7 @@ typedef enum BlockOpType { > BLOCK_OP_TYPE_RESIZE, > BLOCK_OP_TYPE_STREAM, > BLOCK_OP_TYPE_REPLACE, > + BLOCK_OP_TYPE_BACKING_REFERENCE, > BLOCK_OP_TYPE_MAX, > } BlockOpType; > =20 > diff --git a/include/block/block_int.h b/include/block/block_int.h > index 08dd8ba..624945d 100644 > --- a/include/block/block_int.h > +++ b/include/block/block_int.h > @@ -375,6 +375,7 @@ struct BlockDriverState { > QDict *full_open_options; > char exact_filename[PATH_MAX]; > =20 > + bool backing_reference; > BlockDriverState *backing_hd; > BlockDriverState *file; > =20 > diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051 > index 0360f37..fd67f40 100755 > --- a/tests/qemu-iotests/051 > +++ b/tests/qemu-iotests/051 > @@ -116,6 +116,19 @@ run_qemu -drive file=3D"$TEST_IMG",file.backing.driv= er=3Dfile,file.backing.filename=3D > run_qemu -drive file=3D"$TEST_IMG",file.backing.driver=3Dqcow2,file.back= ing.file.filename=3D"$TEST_IMG.orig" > =20 > echo > +echo =3D=3D=3D Backing file reference =3D=3D=3D > +echo > + > +run_qemu -drive file=3D"$TEST_IMG",if=3Dnone,id=3Ddrive0 \ > + -drive file=3D"$TEST_IMG",driver=3Dqcow2,backing_reference.drive_id= =3Ddrive0,backing_reference.hidden-disk.filename=3D"$TEST_IMG.hidden" > + > +run_qemu -drive file=3D"$TEST_IMG",if=3Dnone,id=3Ddrive0 \ > + -drive file=3D"$TEST_IMG",driver=3Dqcow2,backing_reference.drive_id= =3Ddrive0,backing_reference.hidden-disk.filename=3D"$TEST_IMG.hidden",backi= ng.file.filename=3D"$TEST_IMG.orig" > + > +run_qemu -drive file=3D"$TEST_IMG",if=3Dnone,id=3Ddrive0 \ > + -drive file=3D"$TEST_IMG",driver=3Dqcow2,file.backing_reference.driv= e_id=3Ddrive0,file.backing_reference.hidden-disk.filename=3D"$TEST_IMG.hidd= en",file.backing.file.filename=3D"$TEST_IMG.orig" > + > +echo > echo =3D=3D=3D Enable and disable lazy refcounting on the command line, = plus some invalid values =3D=3D=3D > echo > =20 > diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out > index 2890eac..cb8340b 100644 > --- a/tests/qemu-iotests/051.out > +++ b/tests/qemu-iotests/051.out > @@ -75,6 +75,19 @@ Testing: -drive file=3DTEST_DIR/t.qcow2,file.backing.d= river=3Dqcow2,file.backing.fil > QEMU_PROG: -drive file=3DTEST_DIR/t.qcow2,file.backing.driver=3Dqcow2,fi= le.backing.file.filename=3DTEST_DIR/t.qcow2.orig: Driver doesn't support ba= cking files > =20 > =20 > +=3D=3D=3D Backing file reference =3D=3D=3D > + > +Testing: -drive file=3DTEST_DIR/t.qcow2,if=3Dnone,id=3Ddrive0 -drive fil= e=3DTEST_DIR/t.qcow2,driver=3Dqcow2,backing_reference.drive_id=3Ddrive0,bac= king_reference.hidden-disk.filename=3DTEST_DIR/t.qcow2.hidden > +QEMU X.Y.Z monitor - type 'help' for more information > +(qemu) q=1B[K=1B[Dqu=1B[K=1B[D=1B[Dqui=1B[K=1B[D=1B[D=1B[Dquit=1B[K > + > +Testing: -drive file=3DTEST_DIR/t.qcow2,if=3Dnone,id=3Ddrive0 -drive fil= e=3DTEST_DIR/t.qcow2,driver=3Dqcow2,backing_reference.drive_id=3Ddrive0,bac= king_reference.hidden-disk.filename=3DTEST_DIR/t.qcow2.hidden,backing.file.= filename=3DTEST_DIR/t.qcow2.orig > +QEMU_PROG: -drive file=3DTEST_DIR/t.qcow2,driver=3Dqcow2,backing=3Ddrive= 0,backing.file.filename=3DTEST_DIR/t.qcow2.orig: Option "backing_reference.= *" and "backing.*" cannot be used together > + > +Testing: -drive file=3DTEST_DIR/t.qcow2,if=3Dnone,id=3Ddrive0 -drive fil= e=3DTEST_DIR/t.qcow2,driver=3Dqcow2,file.backing_reference.drive_id=3Ddrive= 0,file.backing_reference.hidden-disk.filename=3DTEST_DIR/t.qcow2.hidden,fil= e.backing.file.filename=3DTEST_DIR/t.qcow2.orig > +QEMU_PROG: -drive file=3DTEST_DIR/t.qcow2,driver=3Dqcow2,file.backing=3D= drive0,file.backing.file.filename=3DTEST_DIR/t.qcow2.orig: Option "backing_= reference.*" and "backing.*" cannot be used together > + > + > =3D=3D=3D Enable and disable lazy refcounting on the command line, plus = some invalid values =3D=3D=3D > =20 > Testing: -drive file=3DTEST_DIR/t.qcow2,format=3Dqcow2,lazy-refcounts=3D= on > --=20 > 2.1.0 >=20