Am 07.07.2016 um 16:17 hat Kevin Wolf geschrieben: > Am 07.07.2016 um 14:59 hat Alberto Garcia geschrieben: > > On Thu 07 Jul 2016 02:11:27 PM CEST, Kevin Wolf wrote: > > > In order to remove the necessity to use BlockBackend names in the > > > external API, we want to allow node-names everywhere. This converts > > > block-stream to accept a node-name without lifting the restriction that > > > we're operating at a root node. > > > > > > In case of an invalid device name, the command returns the GenericError > > > error class now instead of DeviceNotFound, because this is what > > > qmp_get_root_bs() returns. > > > > > > Signed-off-by: Kevin Wolf <kw...@redhat.com> > > > --- > > > blockdev.c | 32 ++++++++++++++++++++------------ > > > qapi/block-core.json | 5 +---- > > > qmp-commands.hx | 2 +- > > > tests/qemu-iotests/030 | 2 +- > > > 4 files changed, 23 insertions(+), 18 deletions(-) > > > > > > diff --git a/blockdev.c b/blockdev.c > > > index 0f8065c..01e57c9 100644 > > > --- a/blockdev.c > > > +++ b/blockdev.c > > > @@ -1172,6 +1172,23 @@ fail: > > > return dinfo; > > > } > > > > > > +static BlockDriverState *qmp_get_root_bs(const char *name, Error **errp) > > > +{ > > > + BlockDriverState *bs; > > > + > > > + bs = bdrv_lookup_bs(name, name, errp); > > > + if (bs == NULL) { > > > + return NULL; > > > + } > > > + > > > + if (!bdrv_has_blk(bs)) { > > > + error_setg(errp, "Need a root block node"); > > > + return NULL; > > > + } > > > > Since b6d2e59995f when you create a block job a new BlockBackend is > > created on top of the BDS. What happens with this check if we allow > > creating jobs in a non-root BDS? > > Hm, you mean I'd start first an intermediate streaming job and then I > can call commands on the target node that I shouldn't be able to call? > It's a good point, but I think op blockers would prevent that this > actually works. > > If we wanted to keep exactly the old set of nodes that is allowed, we > could make qmp_get_root_bs() look for a _named_ BlockBackend. But that > would kind of defeat the purpose of this series, which wants to allow > these commands on named nodes that are directly used for -device. > > Another option - and maybe that makes more sense than the old rule > anyway because you already can have a BB anywhere in the middle of the > graph - would be to check that the node doesn't have any non-BB parent.
This is what I'm implementing now. The reason for this is that bdrv_has_blk() obviously rejects configurations where you have only a node name, but no BB. And the whole point of the series is to move towards a model without named BBs, so this would mean that you can only run block job on attached nodes, which doesn't make a lot of sense (and gives qemu-iotests some trouble). With this option implemented, a node that isn't attached anywhere can be used for root node commands, as it should. Kevin