[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH 3/3] block: mirror - zero unallocat
From: |
Jeff Cody |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH 3/3] block: mirror - zero unallocated target sectors when zero init not present |
Date: |
Wed, 30 Sep 2015 10:43:50 -0400 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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
- Re: [Qemu-block] [Qemu-devel] [PATCH 3/3] block: mirror - zero unallocated target sectors when zero init not present, Kevin Wolf, 2015/10/01
- Re: [Qemu-block] [Qemu-devel] [PATCH 3/3] block: mirror - zero unallocated target sectors when zero init not present, Paolo Bonzini, 2015/10/01
- Re: [Qemu-block] [Qemu-devel] [PATCH 3/3] block: mirror - zero unallocated target sectors when zero init not present, Kevin Wolf, 2015/10/03
- Re: [Qemu-block] [Qemu-devel] [PATCH 3/3] block: mirror - zero unallocated target sectors when zero init not present, Paolo Bonzini, 2015/10/04
- Re: [Qemu-block] [Qemu-devel] [PATCH 3/3] block: mirror - zero unallocated target sectors when zero init not present,
Jeff Cody <=
- Re: [Qemu-block] [Qemu-devel] [PATCH 3/3] block: mirror - zero unallocated target sectors when zero init not present, Kevin Wolf, 2015/10/08
- Re: [Qemu-block] [Qemu-devel] [PATCH 3/3] block: mirror - zero unallocated target sectors when zero init not present, Jeff Cody, 2015/10/08
- Re: [Qemu-block] [Qemu-devel] [PATCH 3/3] block: mirror - zero unallocated target sectors when zero init not present, Paolo Bonzini, 2015/10/08
- Re: [Qemu-block] [Qemu-devel] [PATCH 3/3] block: mirror - zero unallocated target sectors when zero init not present, Kevin Wolf, 2015/10/08
- Re: [Qemu-block] [Qemu-devel] [PATCH 3/3] block: mirror - zero unallocated target sectors when zero init not present, Paolo Bonzini, 2015/10/08