I feel bad for trimming your mail so much, but that's just because I've read it and I agree. So, it's trimming of the good kind. (Then again, when is trimming of a long mail ever bad?)
On 16.11.18 18:13, Kevin Wolf wrote: [...] > What we noted down about counters was more meaningful progress because > we think this can actually provide the semantics that we need. Of > course, there is still some hand-waving involved (we'll just check the > counter when modifying the graph), which might not be that trivial to > implement because bdrv_replace_child() can't return an error... Sure, there is much to talk about there. You're right, we probably need a transaction system there, too, which makes things not so simple again. And then there's the fact that bdrv_detach_child() currently cannot fail, and we really don't want it to fail in e.g. bdrv_close(). Expressing that may be tricky, too. >>> It doesn't feel too bad to me, but that's subjective. >> >> It's not worse than quiescing/draining, that's for sure. > > How would _you_ know? :-) I seem to recall to at least have read your patches... ;-) [...] > For the implementation with counters, maybe we actually need two of them > like perm/shared in the permission system. One that means that we're > going to modify the graph, and the other one that means that we can't > tolerate graph modifications. > > For the public interface, we might actually treat it mostly like > permissions, just edge permissions, not node permissions. Hm. I see your point. But having a "perm" field which would allow a party to be able to do graph modifications for as long as it has claimed that field would mean that the probably also want other parties not to modify that exact same part of the graph in the meantime. Like commit which will modify part of the graph while over the whole lifetime of the job it doesn't want that part to be modified by something else. But then again, exactly that "perm" field solves the issue, too. If you are required to take the permission (and in the process block other modifications on that edge) before doing graph modifications, the check function can simply see that there is exactly one blocker, and infer that this must be the party doing the modification itself. >> So, no, I do not have a good technical counter-argument. But my >> argument of complexity still stands after having gone through >> understanding how GRAPH_MOD might actually work for at least the third >> time now, without it ever having made the next time any easier. >> >> Even if GRAPH_MOD were correctly implemented tomorrow, in a way >> conforming to what we've discussed now, I have the strong feeling that I >> still would get to a point where I forgot its meaning and would have to >> go through all of this again. >> >> And that's completely disregarding people new to the codebase. > > As I said, it's moot anyway. It doesn't provide the right semantics. True, but the discussion itself isn't moot to me. I wanted to stress that given something that is rather hard to grasp, I prefer to throw it away as long as it is not functional yet, whenever a more intuitive alternative presents itself. Max
signature.asc
Description: OpenPGP digital signature
