The Saturday 21 Jun 2014 à 16:53:58 (+0800), Fam Zheng wrote : > On Fri, 06/20 09:30, Eric Blake wrote: > > On 06/19/2014 11:01 PM, Fam Zheng wrote: > > > On Thu, 06/19 22:20, Benoît Canet wrote: > > >> The Thursday 19 Jun 2014 à 14:13:20 (-0600), Eric Blake wrote : > > >>> On 06/19/2014 02:01 PM, Benoît Canet wrote: > > >>>> As the code will start to operate on arbitratry nodes we need the op > > >>>> blocker > > >>> > > >>> s/arbitratry/arbitrary/ > > >>> > > >>>> to recursively block or unblock whole BDS subtrees. > > > > > > I don't get the reason, can you elaborate? > > > > Consider what happens if I have: > > > > base <- snap1 <- active > > > > then I start a fleecing NBD server on the state as it was at snap1: > > > > base <- snap1 <- active > > \- fleecing > > > > then I do a blockpull into active: > > > > base <- snap1 <- fleecing > > active > > > > at this point, base and snap1 are no longer tied to active, but they > > STILL must be protected from operations that would modify their contents > > in a way that would break the fleecing operation. The solution we are > > looking at is making BDS blockers recursive to every element of the > > chain, not just the top-level device. > > This would already have been protected by backing blocker of fleecing target. > > > > > Another example: consider: > > > > base <- snap1 <- active > > > > then someone uses Jeff's proposed new change-backing-file QMP command to > > rewrite the snap1 metadata to point to base via a relative name instead > > of an absolute name. It shouldn't matter whether active is blocked, but > > only whether snap1 is blocked. But to know if snap1 is blocked, we have > > to propagate the blockers of active down recursively to its backing files. > > Why do we need to block changging of metadata? I think this operation is safe > in most cases. > > Correct me if I'm missing anything, but even if snap1 _is_ blocked, it would > be > because snap1 is serving as backing of active. In this case, the actual > blocker > should be active->backing_blocker. > > > > > >> What would be a cleaner solution ? > > > > > > What is the question to solve? > > > > I think Jeff's idea is on target - rather than blocking by operation, we > > should instead be blocking on access patterns (various operations > > trigger several access patterns): > > https://lists.gnu.org/archive/html/qemu-devel/2014-06/msg04752.html > > > > Jeff's initial list included: > > > > > So if I think of operations that are done on block devices from a > > > block job, and chuck them into categories, I think we have: > > > > > > 1) Read of guest-visible data > > > 2) Write of guest-visible data > > > 3) Read of host-visible data (e.g. image file metadata) > > > 4) Write of host-visible data (e.g. image file metadata, such as > > > the backing-file) > > > 5) Block chain manipulations (e.g. movement of a BDS, change to r/w > > > instead of r/o, etc..) > > > 6) I/O attribute changes (e.g. throttling, etc..) > > Most operations looks safe to me, given the way how IOThreads and coroutine > work now. It's only the chain manpulations in long running block jobs that are > exclusive, and by nature it should be checked per chain. Can we set some op > blockers on the bottom BDS and check it each time, to prevent user from > starting a second chain manipulator?
I don't know if bottom BDS locking is any good because some driver like quorum have multiple childs. Locking everytime the root (top) of the tree seems a feasible solution indeed. Best regards Benoît > > Fam >
