[Top][All Lists]

[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

From: Paolo Bonzini
Subject: Re: [Qemu-block] block: Dirty bitmaps and COR in bdrv_move_feature_fields()
Date: Mon, 29 Feb 2016 15:49:13 +0100
User-agent: 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.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]