On Sun 11 Nov 2018 11:25:00 PM CET, Max Reitz wrote: > Permission system > ================= > > GRAPH_MOD > --------- > > We need some way for the commit job to prevent graph changes on its > chain while it is running. Our current blocker doesn’t do the job, > however. What to do? > > - We have no idea how to make a *permission* work. Maybe the biggest > problem is that it just doesn’t work as a permission, because the > commit job doesn’t own the BdrvChildren that would need to be > blocked (namely the @backing BdrvChild).
What I'm doing at the moment in my blockdev-reopen series is check whether all parents of the node I want to change share the GRAPH_MOD permission. If any of them doesn't then the operation fails. This can be checked by calling bdrv_get_cumulative_perm() or simply bdrv_check_update_perm(..., BLK_PERM_GRAPH_MOD, ...). It solves the problem because e.g. the stream block job only allows BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED, so no graph modifications allowed. > (We never quite knew what “taking the GRAPH_PERMISSION” or > “unsharing the GRPAH_MOD permission” was supposed to mean. Figuring > that out always took like half an our in any face-to-face meeting, > and then we decided it was pretty much useless for any case we had > at hand.) Yeah it's a bit unclear. It can mean "none of this node's children can be replaced / removed", which is what I'm using it for at the moment. > Reopen > ------ > > How should permissions be handled while the reopen is under way? > Maybe we should take the union of @perm before and after, and the > intersection of @shared before and after? Do you have an example of a case in which you're reopening a node and the change of permissions causes a problem? > - Is it possible that changing an option may require taking an > intermediate permission that is required neither before nor after > the reopen process? > Changing a child link comes to mind (like changing a child from one > BDS to another, where the visible data changes, which would mean we > may want to e.g. unshare CONSISTENT_READ during the reopen). > However: > 1. It is unfeasible to unshare that for all child changes. > Effectively everything requires CONSISTENT_READ, and for good > reason. > 2. Why would a user even change a BDS to something of a different > content? > 3. Anything that currently allows you to change a child node assumes > that the user always changes it to something of the same content > (some take extra care to verify this, like mirror, which makes > sure that @replaces and the target are connected, and there are > only filter nodes in between). I don't think the user wants to change a BDS to something of a different content, but it may happen that QEMU doesn't have a way to verify whether the content is the same or not. I think we have one such case already with 'blockdev-snapshot', in which you add a BDS with blockdev-add and then add its contents on top of an existing BDS. Berto