Re: [Qemu-devel] [PATCH V1 1/2] Implement sync modes for drive-backup.

[Paolo Bonzini]

Il 15/07/2013 19:49, Ian Main ha scritto:
> OK well, I'll explain here my understanding.  I apologize if I explain
> more than needed but it might be good to get this out there anyway.

No problem, it's better to be verbose than to have an extra iteration.

> When we do the create with:
> 
> bdrv_img_create(target, format, source->filename,                             
>                                                                               
>                                       
>                 source->drv->format_name, NULL,                               
>                                                                               
>                                       
>                 size, flags, &local_err, false);                 
> 
> We are creating a new target image using the same backing file as the
> original disk image.  Then any new data that has been laid on top of it
> since creation is copied in the main backup_run() loop.  There is an
> extra check in the 'TOP' case so that we don't bother to copy all the
> data of the backing file as it already exists in the target.  This is
> where the bdrv_co_is_allocated() is used to determine if the data exists
> in the topmost layer or below.

Yes, so far I understood it.

>> I'm not sure how the "none" mode works with these patches.  It's quite
>> possible I'm wrong, of course, but then the explanation should be in the
>> code or the commit message.  It should be in the code or the commit
>> message even if my suggestions are applied. :)
> 
> For mode 'NONE' we create the new target image and only copy in the
> original data from the disk image starting from the time the call was
> made.  This preserves the point in time data by only copying the parts
> that are *going to change* to the target image.  This way we can
> reconstruct the final image by checking to see if the given block exists
> in the new target image first, and if it does not, you can get it from
> the original image.  This is basically an optimization allowing you to
> do point-in-time snapshots with low overhead vs the 'FULL' version.
> 
> Since there is no old data to copy out the loop in backup_run() for the
> NONE case just calls qemu_coroutine_yield() which only wakes up after an
> event (usually cancel in this case).  The rest is handled by the
> before_write notifier which again calls backup_do_cow() to write out the
> old data so it can be preserved.
> 
> Does that help?

Yes, it helps---I think I'm right. :)

For mode 'NONE' we only copy in the original data from the disk image,
but what makes us read the old image for the sectors we haven't copied
yet?  For that to work, we need to set target_bs->backing_hd.

Now, the question is whether to do it for mode 'NONE' only, or also for
the others.  It is certainly required for mode 'NONE', because this mode
will never get complete data in the destination.  But it may actually be
a good idea for other sync modes.  This is because block-backup is a
sort of live snapshot, except that the active image remains the
pre-snapshot one.  Thus it makes sense to have the same defaults as
blockdev-snapshot-live, including making "qcow2" the default format.

A user that wants to use an NBD client target for backup (this is
different from fleecing, which uses a temporary file + an NBD server)
should specify a "raw" format anyway, independent of the default we
choose, so this change doesn't affect other use cases.

On top of this, target_bs->backing_hd needs to be set manually to bs
itself during the copy (*even if the source used in bdrv_img_create is
bs->backing_hd, or is NULL!*), and reset just before closing target_bs.
 This way, if the target is exposed via the NBD server, it reads as a
full copy of the image even while the backup is ongoing.  This can be
done using BDRV_O_NO_BACKING.

Paolo