On Mon, Feb 27, 2012 at 1:47 PM, Paolo Bonzini <pbonz...@redhat.com> wrote: > On 02/27/2012 02:09 PM, Stefan Hajnoczi wrote: >> > Non-incremental mode is "pre-copy" migration. It would stream in the >> > background from the source to the destination. In this case: >> > >> > - you need to differentiate streaming writes from other writes. When >> > streaming, you do not want to issue spurious writes to the source; >> > >> > - you can skip streaming anything that has been written to the >> > destination already. This means that you need: 1) a bitmap similar to >> > is_allocated; 2) to widen writes to a cluster; 3) serialization similar >> > to copy-on-read. >> > >> > I'm not yet sure how much of this will be generalized in the block layer >> > and block/stream.c, and how much will be specific to blkmirror, but in >> > general none of this applies to blkverify. >> >> Agreed but I'm not sure it has to do with blkmirror either. blkmirror >> and blkverify perform the same function except that blkverify mirrors >> reads too and checks that they match. >> >> Who knows when and if pre-copy will be (re)implemented, it's not a >> good argument to say let's duplicate block mirroring because we're not >> sure about the design of a feature feature yet which might want to >> hook in here. > > It's not a duplicate, it just happens that two very simple drivers share > 1 operation out of 4 (open, read, write, flush). There are other > differences, for example: > > 1) blkverify hardcodes raw for one format and auto-detects the other. > blkmirror needs to have a specified format for both, and I don't think > starting another bikeshedding discussion on blkverify backwards > compatibility is a good idea;
Backwards compatibility isn't needed here, it's a debugging tool and some distros even disable it. Allowing another format for the reference image is a feature that would be nice to have. It allows direct qcow2 to vmdk comparison, for example, if we ever hit a bug that benefits from this type of comparison. > 2) blkverify doesn't flush the raw file; It's fine to flush the reference file. This is an accidental difference :). > 3) blkverify in the future could add callbacks to create snapshots or > load/save imgstate, and forward them to the test file; this doesn't make > sense for blkmirror. I guess what I'm saying is, if we need to copy-paste in order to fork them in the future that's fine, but why maintain duplicates in the mean-time? Please make the codebase nice today. We can always extend it in the future, we're not forced to keep them unified forever if it turns out we want to split them. But as it stands they are essentially the same thing. Stefan
