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

[Kevin Wolf <kwolf@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2710 bytes --]

Am 28.09.2015 um 19:32 hat Max Reitz geschrieben:
> On 28.09.2015 05:29, Jeff Cody wrote:
> > During mirror, if the target device does not have support zero
> > initialization, a mirror may result in a corrupt image.
> > 
> > For instance, on mirror to a host device with format = raw, whatever
> > random data is on the target device will still be there for unallocated
> > sectors.
> > 
> > This is because during the mirror, we set the dirty bitmap to copy only
> > sectors allocated above 'base'.  In the case of target devices where we
> > cannot assume unallocated sectors will be read as zeroes, we need to
> > explicitely zero out this data.
> > 
> > In order to avoid zeroing out all sectors of the target device prior to
> > mirroring, we do zeroing as part of the block job.  A second dirty
> > bitmap cache is created, to track sectors that are unallocated above
> > 'base'.  These sectors are then checked for status of BDRV_BLOCK_ZERO
> > on the target - if they are not, then zeroes are explicitly written.
> > 
> > This only occurs under two conditions:
> > 
> >     1. 'mode' != "existing"
> >     2. bdrv_has_zero_init(target) == NULL
> > 
> > We perform the mirroring through mirror_iteration() as before, except
> > in two passes.  If the above two conditions are met, the first pass
> > is using the bitmap tracking unallocated sectors, to write the needed
> > zeroes.  Then, the second pass is performed, to mirror the actual data
> > as before.
> > 
> > If the above two conditions are not met, then the first pass is skipped,
> > and only the second pass (the one with the actual data) is performed.
> > 
> > Signed-off-by: Jeff Cody <jcody@redhat.com>

> > @@ -737,6 +787,7 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
> >          return;
> >      }
> >  
> > +    s->zero_unallocated = !existing && !bdrv_has_zero_init(target);
> 
> I think this should be set only if we're doing a full mirror operation.
> For instance, I could do a none, top or incremental mirror to a new
> qcow2 file, which would give it a backing file, obviously. You're lucky
> in that qcow2 claims to always have zero initialization, when this is in
> fact not true (someone's ought to fix that...): With a backing file, an
> overlay file just cannot have zero initialization, it's impossible
> (well, unless the backing file is completely zero).

bdrv_has_zero_init() takes care of that, in theory. The "problem" here
is that the target is opened with BDRV_O_NO_BACKING, so the block layer
doesn't consider this an image with a backing file.

Is there anything better than bs->backing_hd that we could check?

Kevin

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]