On 29/02/2016 15:36, Kevin Wolf wrote: > Hi all, > > I'm currently trying to get rid of bdrv_move_feature_fields(), so we can > finally have more than one BB per BDS. Generally the way to do this is > to move features from BDS and block.c to BB and block-backend.c. > However, for two of the features I'm not sure about this: > > * Copy on Read: > > When Jeff introduced bdrv_append() in commit 8802d1fd, the CoR flag > was already moved to the new top level when taking a snapshot. Does > anyone remember why it works like that? It doesn't seem to make a lot > of sense to me. > > The use case for manually enabled CoR is to avoid reading data twice > from a slow remote image, so we want to save it to a local overlay, > say an ISO image accessed via HTTP to a local qcow2 overlay. When > taking a snapshot, we end up with a backing chain like this: > > http <- local.qcow2 <- snap_overlay.qcow2 > > There is no point in performing copy on read from local.qcow2 into > snap_overlay.qcow2, we just want to keep copying data from the remote > source into local.qcow2. > > Possible caveat: We would be writing to a backing file, but that's > similar to what some block jobs do, so if we design our op blockers to > cover this case, it should be fine.
Yes, considering that bdrv_append() didn't reopen read-only it probably was a mistake. > On the other hand, the QMP interface clearly describes bitmaps as > belonging to a node rather than a BB (you can use node-name, even with > no BB attached), so moving them could be considered a bug, even if > it is the existing behaviour. > > I can imagine use cases for both ways, so the interface that would > make the most sense to me is to generally keep BDSes at their node, > and to provide a QMP command to move them to a different one. I'm not sure if anyone has actually ever used dirty bitmaps except the Parallel folks. I think it makes sense to keep them in the node. At the time I added the code to bdrv_move_feature_fields(), the reasoning was probably that no one expected dirty bitmaps except on the top BDS, and there were no blockers, so anything else would have caused bugs. Nowadays, with blockers _and_ user bitmaps, it's probably best not to move the dirty bitmaps. Paolo
