Re: [Qemu-devel] [PATCH 0/2] introduce bdrv_swap, implement bdrv_append on top

[Stefan Hajnoczi]

On Fri, Jun 15, 2012 at 10:38 AM, Kevin Wolf <address@hidden> wrote:
> Am 14.06.2012 16:55, schrieb Paolo Bonzini:
>> Yet another tiny bit extracted from block mirroring, looks like it
>> should be useful for block commit too.
>>
>> Paolo Bonzini (2):
>>   block: copy over job and dirty bitmap fields in bdrv_append
>>   block: introduce bdrv_swap, implement bdrv_append on top of it
>>
>>  block.c |  175 
>> +++++++++++++++++++++++++++++++++++++--------------------------
>>  block.h |    1 +
>>  2 files changed, 103 insertions(+), 73 deletions(-)
>>
>
> I was really hoping we could get rid of bdrv_append() rather than extend
> it and spread its use...
>
> What exactly do we need this for? I'm sure you have good reasons, but
> with such hackish approaches the justification should be explicit.

Opening twice and switching to the new state is an okay approach.
It's just made very difficult because BlockDriverState has grown into
a monster struct.  So the implementation is ugly.

If we split BlockDriverState (e.g. guest-visible block device state vs
block driver state) can we do away with the in-place swap?  If it's
possible to implement this using regular pointer assignment instead of
swapping fields, then this approach will become reasonable IMO.

Stefan