On Wed 13 Mar 2019 03:29:36 PM CET, Kevin Wolf wrote: > Am 13.03.2019 um 15:14 hat Alberto Garcia geschrieben: >> On Tue 12 Mar 2019 05:12:53 PM CET, Vladimir Sementsov-Ogievskiy wrote: >> >> + /* This function changes all links that point to top and makes >> >> + * them point to base. Check that none of them is frozen. */ >> >> + QLIST_FOREACH(c, &top->parents, next_parent) { >> >> + if (c->frozen) { >> >> + goto exit; >> >> + } >> >> + } >> >> + >> >> > Hmm, looking at this I have a bit unrelated questions about >> > bdrv_drop_intermediate: >> > >> > 1. if we fail on some iteration of QLIST_FOREACH_SAFE loop, we finish >> > up with partly updated graph?? >> >> > I don't see any kind of roll-back in this case.. Why it don't >> > operate like bdrv_replace_node, which firstly check all >> > permissions, and then update all parents in fail-free loop? >> >> I think you're right. If you don't have something written already I >> could try to do it. >> >> > 2. And therefore, why we just don't call bdrv_replace_node(top, base, >> > errp) from bdrv_drop_intermediate? >> >> I think that would not call role->update_filename(). > > And role->update_filename() involves I/O, so you can't roll back > across it anyway. I think this was the reason why we didn't roll back > the in-memory state either, it would become inconsistent with what is > on the disk.
But I think at least for the bdrv_check_update_perm() case we should call bdrv_abort_perm_update() on all nodes if things fail, and we're currently not doing it. Berto