On Tue, Sep 29, 2015 at 12:52:33PM +0200, Paolo Bonzini wrote: > > > On 29/09/2015 11:35, Kevin Wolf wrote: > > The caller could be copying the backing file in the background and it > > may not yet be finished. > > Yes, and this is permitted (the destination file of mirroring is opened > with BDRV_O_NO_BACKING). > > Some more assumptions arise when block-job-complete is invoked, because > at this point the content must not change under the guest's feet. > Because block-job-complete does bdrv_open_backing_file on the > destination, for sync!='full' it means that either 1) the image has no > backing file, but it starts with the content of the backing file or 2) > the image's backing file is complete at the time block-job-complete is > invoked. > > For mode!='existing' it is always case (2), and the backing file is > complete all the time; for mode=='existing' the backing file could be > copied in the background, and case (1) could happen as well. An example > of case (1) is replacing sync=='full' with a "fast copy" of the backing > file (e.g. via btrfs's COW copies) and sync=='top'. This should be valid.
One issue is that QEMU will do mode!='existing' && sync!='full' for drivers that do not support backing files (raw host devices, for instance). We could refuse to start a mirror in the case of: mode != 'existing' && sync != 'full' && !target->drv->supports_backing Alternatively, we could do the two-pass zero approach in this patch, except under the following conditions: sync == 'full' || (mode != 'existing' && !target->drv->supports_backing) (In the sync == 'full' case, we could also just queue all sectors, as Kevin suggested) > > Of course, if block-job-complete is never called, all bets are off. > > > We don't do this now, but assuming > > the promise means that we could e.g. read the backing file in order to > > optimise sparseness in the target (if it happens to have the same data > > as its backing file) - and I don't think this would be valid with our > > currently documented API. > > Accessing the backing file of the target is never valid indeed. > > > Anyway, the conclusion that we shouldn't zero unrelated sectors is still > > right. But it's because we document which sectors we copy, not because > > we can make assumptions about the user. > > Right. > > Paolo