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
