[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 2/3] docs/interop/bitmaps: use blockdev-backup
From: |
Kashyap Chamarthy |
Subject: |
Re: [PATCH v2 2/3] docs/interop/bitmaps: use blockdev-backup |
Date: |
Thu, 6 May 2021 11:34:27 +0200 |
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
[PATCH v2 2/3] docs/interop/bitmaps: use blockdev-backup, Vladimir Sementsov-Ogievskiy, 2021/05/05
[PATCH v2 1/3] docs/block-replication: use blockdev-backup, Vladimir Sementsov-Ogievskiy, 2021/05/05