Re: [PATCH v2 2/3] docs/interop/bitmaps: use blockdev-backup

[Kashyap Chamarthy <kchamart@redhat.com>]

On Wed, May 05, 2021 at 04:58:02PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> We are going to deprecate drive-backup, so use modern interface here.
> In examples where target image creation is shown, show blockdev-add as
> well. If target creation omitted, omit blockdev-add as well.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  docs/interop/bitmaps.rst | 285 +++++++++++++++++++++++++++++----------
>  1 file changed, 215 insertions(+), 70 deletions(-)

Looks fine to me.  I have a couple of nits below, perhaps whoever is
applying the patch can tweak them.  FWIW:

Reviewed-by: Kashyap Chamarthy <kchamart@redhat.com>    

> diff --git a/docs/interop/bitmaps.rst b/docs/interop/bitmaps.rst
> index 059ad67929..ef95090c81 100644
> --- a/docs/interop/bitmaps.rst
> +++ b/docs/interop/bitmaps.rst
> @@ -539,12 +539,11 @@ other partial disk images on top of a base image to reconstruct a full backup
>  from the point in time at which the incremental backup was issued.
>  
>  The "Push Model" here references the fact that QEMU is "pushing" the modified
> -blocks out to a destination. We will be using the `drive-backup
> -<qemu-qmp-ref.html#index-drive_002dbackup>`_ and `blockdev-backup
> -<qemu-qmp-ref.html#index-blockdev_002dbackup>`_ QMP commands to create both
> +blocks out to a destination. We will be using the  `blockdev-backup
> +<qemu-qmp-ref.html#index-blockdev_002dbackup>`_ QMP command to create both
>  full and incremental backups.
>  
> -Both of these commands are jobs, which have their own QMP API for querying and
> +The command is a job, which has its own QMP API for querying and

nit: s/job/background job/

>  management documented in `Background jobs
>  <qemu-qmp-ref.html#Background-jobs>`_.
>  
> @@ -557,6 +556,10 @@ create a new incremental backup chain attached to a drive.
>  This example creates a new, full backup of "drive0" and accompanies it with a
>  new, empty bitmap that records writes from this point in time forward.
>  
> +The target may be created with help of `blockdev-add

It is less ambiguous (at least to my eyes) to replace "may" with "can".

nit: s/with help of/with the help of/

> +<qemu-qmp-ref.html#index-blockdev_002dadd>`_ or `blockdev-create
> +<qemu-qmp-ref.html#index-blockdev_002dcreate>`_ command.
> +
>  .. note:: Any new writes that happen after this command is issued, even while
>            the backup job runs, will be written locally and not to the backup
>            destination. These writes will be recorded in the bitmap
  
[...]

-- 
/kashyap