[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH] drive-backup 'stream' mode
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [RFC PATCH] drive-backup 'stream' mode |
Date: |
Mon, 14 Oct 2013 14:24:13 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.0 |
On 10/14/2013 02:10 PM, Wolfgang Richter wrote:
>>
>> 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?
http://wiki.qemu.org/Contribute/SubmitAPatch suggests submitting a new
top-level thread for each revision of a patch series, along with a
changelog after the --- or in the cover letter to help focus reviewers
on what changed from the earlier revision.
>
>>
>>> #
>>> # 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.
No - drive-mirror and drive-backup are independent, and both useful.
Each fills a niche that the other cannot.
>
> 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.
'drive-backup' creates a point-in-time up front.
'drive-mirror' can be used to create a point-in-time at the tail end
(when you gracefully cancel the job once it is in mirroring phase). But
it also does not have to be canceled - as long as it is still running,
you are still mirroring data.
>
> 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.
You can't break the existing semantics, but if you think you can unify
the code base, be my guest.
>
> 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).
>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature