[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] block: Dirty bitmaps and COR in bdrv_move_feature_field
Re: [Qemu-block] block: Dirty bitmaps and COR in bdrv_move_feature_fields()
Mon, 29 Feb 2016 15:49:13 +0100
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.0
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.