On Thu 07 Jul 2016 04:17:21 PM CEST, Kevin Wolf wrote: >> > +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.
Yes, they would but a) the user would get a misleading error message ("you cannot start that job because the device is temporarily being used" rather than "you cannot start that block job there at all"). Probably not so important. b) all the code after the qmp_get_root_bs() call would run under the (incorrect) assumption that the node is a root BDS. This probably doesn't break anything at the moment but leaves a door open for surprises. > 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 would restrict some cases that are possible today. Which ones? Berto