Re: [Qemu-devel] [RFC PATCH] drive-backup 'stream' mode

[Wolfgang Richter]

On Fri, Oct 11, 2013 at 11:38 AM, Eric Blake <address@hidden> wrote:
> On 10/11/2013 09:18 AM, Wolfgang Richter wrote:
>> Idea: Introduce a mode for drive-backup that duplicates writes to
>> another target, not CoW.  It is useful for introspecting (my use
>> case), and for keeping a remote block device in sync with writes
>> (helps with migration or backup).
>>
>>
>
>>
>> This is based off of v1.6.0 code.
>
> Best to rebase it against latest qemu.git.

Done.

>> +++ b/qapi-schema.json
>> @@ -1311,12 +1311,14 @@
>>  #
>>  # @full: copies data from all images to the destination
>>  #
>> -# @none: only copy data written from now on
>> +# @none: only copy on write data written from now on
>> +#
>> +# @stream: copy every new write to target
>
> Add the designation '(since 1.7)' to make it obvious when this mode was
> introduced.

Done.  Is it better to place the updated patch in this thread or start
a new one?

>
>>  #
>>  # Since: 1.3
>>  ##
>>  { 'enum': 'MirrorSyncMode',
>> -  'data': ['top', 'full', 'none'] }
>> +  'data': ['top', 'full', 'none', 'stream'] }
>
> MirrorSyncMode is used by multiple commands; your summary mentions how
> it would affect 'drive-backup', but what happens to 'drive-mirror'?  For
> that matter, why isn't 'drive-mirror' with mode 'none' doing what you
> already want?

Okay, I think my impression might be wrong, but I thought
'drive-mirror' would become deprecated with the new 'drive-backup'
command and code.

If we look at what they do (current documentation and code),
'drive-backup' AFAIK behaves the same for all modes of 'drive-mirror'
_except_ mode 'none' with _better_ consistency guarantees.  That is,
'drive-backup' clearly provides a point-in-time snapshot, whereas
'drive-mirror' may create a point-in-time snapshot, but it can not
guarantee that.

In addition, 'drive-backup's code is cleaner, simpler, and easier to
work with (in my opinion) than 'drive-mirror's code.  This is because
of the new hooks in block.c for tracked requests etc. so that the job
can insert code to be run on every write in a clean manner (I think).

I think that it would be less confusing to subsume 'drive-mirror' into
'drive-backup' so that we have a single command with clear consistency
guarantees, and also it would prevent overloading (and more confusion)
with the meaning of the 'MirrorSyncMode's.

Perhaps a better naming scheme for the modes would then be:

full - as before (same for both commands AFAIK)
top - as before (same for both commands AFAIK)
none - if we only have drive-backup, rename this to 'overlay' as it
creates a low-overhead CoW overlay point-in-time snapshot
stream - either keep my name 'stream' to do what 'none' does for
drive-mirror, or leave this as the 'none' mode with the same
drive-mirror semantics

Thus, I think, with a single extra mode, drive-backup can subsume
drive-mirror.  This reduces the number of commands, the documentation,
and the code (all duplicating each other in some manner).

-- 
Wolf