Am 05.04.2018 um 13:59 hat Paolo Bonzini geschrieben: > On 12/03/2018 12:58, Kevin Wolf wrote: > >> - users of dirty bitmaps would call use perm/shared_perm as in your > >> message above > >> > >> - dirty bitmaps creation calls bdrv_get_cumulative_perm (which should > >> now become public) and checks that it doesn't have BLK_PERM_BYPASS in > >> shared_perm > > > > My proposal was really that users of dirty bitmaps don't change > > anything, but we do everything in the dirty bitmap implementation. Dirty > > bitmap creation would add a BdrvChild with the above permissions. > > Deleting a dirty bitmap would remove the BdrvChild again. > > This is also better because it works if somebody requests > BLK_PERM_BYPASS after dirty bitmap creation. > > However, is there any better way than also creating a dummy BlockDriver > and BlockDriverState? I first thought of a root role similar to > BlockBackend's, but BdrvChildRole doesn't have a way to inject its own > permissions. I then tried moving bdrv_child_perm to BdrvChildRole, and > that almost works except that child_backing has special requirements > (mostly due to commit_top and mirror_top's special block drivers).
Have a look at block_job_add_bdrv(), which does the same thing to restrict permissions while a block job is working on the subchain. Essentially, yes, you'll probably have a new BdrvChildRole (I suppose with .stay_at_node = true and .get_parent_desc only), but the place where you specify the permissions is the bdrv_root_attach_child() call. When you're done, you call bdrv_root_unref_child(). Kevin > Perhaps child_perm should be in BdrvChildRole and BlockDriverState > should only have a bdrv_get_backing_perm (called by > child_backing.child_perm). This makes sense to me since those > permissions are specific to the driver, e.g. whether it has metadata at > all. But this becomes 2.13 material. > > Do you still object to the two opblocker patches? > > Paolo
