| Message ID | 1434617361-17778-7-git-send-email-wency@cn.fujitsu.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Jun 18, 2015 at 04:49:11PM +0800, Wen Congyang wrote: > Signed-off-by: Wen Congyang <wency@cn.fujitsu.com> > Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> > Signed-off-by: Gonglei <arei.gonglei@huawei.com> > --- > block.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/block.c b/block.c > index d1ed227..0b41af4 100644 > --- a/block.c > +++ b/block.c > @@ -1294,6 +1294,14 @@ static int bdrv_open_backing_reference_file(BlockDriverState *bs, > } > > backing_hd = blk_bs(backing_blk); > + /* Don't allow a disk use backing reference target */ > + ret = blk_attach_dev(backing_hd->blk, bs); > + if (ret < 0) { > + error_setg(errp, "backing_hd %s is used by the other device model", > + backing_name); > + goto free_exit; > + } Can you explain the purpose of this patch? I'm not sure blk_attach_dev() is the appropriate API. It is only used by emulated devices but not by the run-time NBD server, for example. This means you are not preventing all other users from accessing this BDS. The op blockers mechanism is normally used to prevent operations on a BDS. See bdrv_op_is_blocked() and bdrv_op_block(). Stefan
At 2015/6/18 20:47, Stefan Hajnoczi Wrote: > On Thu, Jun 18, 2015 at 04:49:11PM +0800, Wen Congyang wrote: >> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com> >> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> >> Signed-off-by: Gonglei <arei.gonglei@huawei.com> >> --- >> block.c | 9 +++++++++ >> 1 file changed, 9 insertions(+) >> >> diff --git a/block.c b/block.c >> index d1ed227..0b41af4 100644 >> --- a/block.c >> +++ b/block.c >> @@ -1294,6 +1294,14 @@ static int bdrv_open_backing_reference_file(BlockDriverState *bs, >> } >> >> backing_hd = blk_bs(backing_blk); >> + /* Don't allow a disk use backing reference target */ >> + ret = blk_attach_dev(backing_hd->blk, bs); >> + if (ret < 0) { >> + error_setg(errp, "backing_hd %s is used by the other device model", >> + backing_name); >> + goto free_exit; >> + } > > Can you explain the purpose of this patch? > > I'm not sure blk_attach_dev() is the appropriate API. It is only used > by emulated devices but not by the run-time NBD server, for example. > This means you are not preventing all other users from accessing this > BDS. > > The op blockers mechanism is normally used to prevent operations on a > BDS. See bdrv_op_is_blocked() and bdrv_op_block(). NBD server will write to this BDS(backing reference target). So I cannot use bdrv_op_block(). If some emulated devices also write to it, the data will be broken. The purpose is that: this BDS cannot be attached, and any emulated devices will meet some erros if it tries to attach to this BDS. Thanks Wen Congyang > > Stefan >
diff --git a/block.c b/block.c index d1ed227..0b41af4 100644 --- a/block.c +++ b/block.c @@ -1294,6 +1294,14 @@ static int bdrv_open_backing_reference_file(BlockDriverState *bs, } backing_hd = blk_bs(backing_blk); + /* Don't allow a disk use backing reference target */ + ret = blk_attach_dev(backing_hd->blk, bs); + if (ret < 0) { + error_setg(errp, "backing_hd %s is used by the other device model", + backing_name); + goto free_exit; + } + /* Backing reference itself? */ if (backing_hd == bs || bdrv_find_overlay(backing_hd, bs)) { error_setg(errp, "Backing reference itself"); @@ -2037,6 +2045,7 @@ void bdrv_close(BlockDriverState *bs) if (backing_hd->backing_hd->job) { block_job_cancel(backing_hd->backing_hd->job); } + blk_detach_dev(backing_hd->backing_hd->blk, bs); bdrv_set_backing_hd(backing_hd, NULL); bdrv_unref(backing_hd->backing_hd); }