[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-block] block: Dirty bitmaps and COR in bdrv_move_feature_fields()
From: |
Kevin Wolf |
Subject: |
[Qemu-block] block: Dirty bitmaps and COR in bdrv_move_feature_fields() |
Date: |
Mon, 29 Feb 2016 15:36:49 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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.
I'm actually pretty sure that simply removing COR from the list, and
therefore changing the behaviour to not move it to the top any more,
is the right thing to do and could be considered a bug fix.
* Dirty bitmaps:
We're currently trying, and if I'm not mistaken failing, to move dirty
bitmaps to the top. The (back then one) bitmap was first added to the
list in Paolo's commit a9fc4408, with the following commit message:
While these should not be in use at the time a transaction is
started, a command in the prepare phase of a transaction might have
added them, so they need to be brought over.
At that point, there was no transactionable command that did this in
the prepare phase. Today we have mirror and backup, but op blockers
should prevent them from being mixed with snapshots in a single
transaction, so I can't see how this change had any effect.
The reason why I think we're failing to move dirty bitmaps to the top
today is that we're moving the head of the list to a different object
without updating the prev link in the first element, so in any case
it's buggy today.
I really would like to keep bitmaps on the BDS where they are, but
unfortunately, we also have user-defined bitmaps by now, and if we
change whether they stick with the top level, that's a change that is
visible on the QMP interface.
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.
With compatibility in mind, this seems to be a reall tough one,
though.
Any comments or ideas how to proceed with those two?
Kevin
- [Qemu-block] block: Dirty bitmaps and COR in bdrv_move_feature_fields(),
Kevin Wolf <=