Re: [Qemu-devel] [PATCH 3/3] block: mirror - zero unallocated target sectors when zero init not present

[Kevin Wolf]

Am 29.09.2015 um 10:42 hat Paolo Bonzini geschrieben:
> 
> 
> On 29/09/2015 10:10, Kevin Wolf wrote:
> >> When mode == 'existing' for a shallow mirror (sync != 'full'),
> >> that is the caller stating that the guest-visible contents of the
> >> destination match the guest-visible contents of the backing
> >> image.  The only sectors to be copied are those that differ from
> >> the backing file, and we should not be zeroing unrelated sectors
> >> because the user has already promised they have the same
> >> guest-visible content as the backing image would report.
> > 
> > Where is this promise documented? I wasn't aware of it and can't
> > seem to find it in the QAPI documentation of drive-mirror.
> 
> I don't think it is really a promise, but it's the only sensible way
> to use this combination.

The caller could be copying the backing file in the background and it
may not yet be finished. So I don't think we can rely on a promise that
isn't explicitly mentioned anywhere. 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.

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.

Kevin

> Paolo
> 
> >> When mode == 'existing' for a full mirror (sync == 'full'), that
> >> is the caller stating that they want every single sector of the
> >> destination written to hold the current state of the source (of
> >> course, allowing for optimizations such as skipping the write
> >> where the contents will read back the same as if the write had
> >> been performed).
> >> 
> >> I think Paolo is right: we care about zeroing unallocated sectors
> >> for sync == 'full', regardless of whether mode == 'existing'.
> > 
> > I agree.
> > 
> >> I also think the reason Jeff confused it for mode == 'existing'
> >> is that the other modes let qemu create the file, but qemu does
> >> not create block devices (the only way to mirror to a block
> >> device is via mode == 'existing'), and it is primarily block
> >> devices where zero init is not guaranteed.
> > 
> > 'qemu-img create' works on block devices (even though for raw it
> > doesn't do more than checking if it's large enough; but for qcow2,
> > it's obvious that it's necessary), so I'm pretty sure that mode !=
> > 'existing' works on them as well.
> > 
> > Kevin
> >