Am 24.08.2026 um 15:37 hat Denis V. Lunev geschrieben: > From: Denis V. Lunev <[email protected]> > > The callback runs after bdrv_reopen_multiple() has committed the > transaction, so it cannot reject the reopen. It can still find that > the node it has just made writable is unusable, and has no way to say > so: bdrv_reopen() returns success and the caller carries on. > > Give it a return value and an Error argument. The reopen stays > committed, the error only reports that the node is gone. Every queued > node still gets its callback, the first error is the one reported. A > callback may leave its node without a driver, and so may the I/O of a > later bdrv_reopen_prepare(), so do not assume that the nodes still > ahead of it in the queue have one. qcow2 is the only implementation and > does not fail yet. > > An error therefore means one of two things now, either that the reopen > was denied and nothing changed, or that it went through and left a tree > which cannot be used. Nothing is undone in the second case: the node is > beyond repair by another reopen, and a caller which reacts to the error > by reopening anything is making it worse. bdrv_reopen_multiple() says > so, nothing else changes. > > Signed-off-by: Denis V. Lunev <[email protected]> > Reviewed-by: Andrey Drobyshev <[email protected]> > CC: Kevin Wolf <[email protected]> > CC: Hanna Reitz <[email protected]> > CC: Andrey Drobyshev <[email protected]> > Cc: [email protected]
What you write both in the commit messages of this and the following patches and in this thread makes me think that you understand well that this approach is fundamentally wrong if we're taking the transaction model seriously, so I don't have to explain that. If we were discussing which transaction phase should contain image repairing completely out of the context of QEMU code, I think we would be very quick to agree that it's the prepare phase. The problem is just that it shows once again the old problem that the reopen design is flawed and doesn't allow writing to the image in the prepare phase for a ro -> rw transition. Even the already existing qcow2_reopen_commit_post() is just a hack; in theory, the bitmaps should be made writable in prepare and then commit should just be a switchover. Reverting to read-only in abort is an operation that can tolerate failure, it just gives up access. I've been thinking for a long time that during reopen, parent nodes should have access to both the old state and the new state of their children to allow all necessary modifications. This feels like a very big change, though. I'm wondering if for ro/rw specifically (which is actually the one setting that is always involved in these nasty cases), it might be enough if we define the expected state after prepare to be that the image is writable if either the old or the new state is writable. Then commit and abort always just either leave it writable or drop write permissions. Do you think this would be a viable approach for your problem here or are there additional problems why the image checking can't be done during prepare? Kevin
