Re: [Qemu-devel] [PATCH v2 10/11] block/backup: support bitmap sync modes for non-bitmap backups

From: Max Reitz <mreitz@redhat.com>
To: John Snow <jsnow@redhat.com>,
	qemu-block@nongnu.org, qemu-devel@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>,
	vsementsov@virtuozzo.com, Markus Armbruster <armbru@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v2 10/11] block/backup: support bitmap sync modes for non-bitmap backups
Date: Tue, 16 Jul 2019 13:45:48 +0200	[thread overview]
Message-ID: <c3db7272-29a9-1626-6937-4cebd4308ffb@redhat.com> (raw)
In-Reply-To: <20190716000117.25219-11-jsnow@redhat.com>


[-- Attachment #1.1: Type: text/plain, Size: 1487 bytes --]

On 16.07.19 02:01, John Snow wrote:
> Accept bitmaps and sync policies for the other backup modes.
> This allows us to do things like create a bitmap synced to a full backup
> without a transaction, or start a resumable backup process.
> 
> Some combinations don't make sense, though:
> 
> - NEVER policy combined with any non-BITMAP mode doesn't do anything,
>   because the bitmap isn't used for input or output.
>   It's harmless, but is almost certainly never what the user wanted.
> 
> - sync=NONE is more questionable. It can't use on-success because this
>   job never completes with success anyway, and the resulting artifact
>   of 'always' is suspect: because we start with a full bitmap and only
>   copy out segments that get written to, the final output bitmap will
>   always be ... a fully set bitmap.
> 
>   Maybe there's contexts in which bitmaps make sense for sync=none,
>   but not without more severe changes to the current job, and omitting
>   it here doesn't prevent us from adding it later.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  block/backup.c       |  8 +-------
>  blockdev.c           | 22 ++++++++++++++++++++++
>  qapi/block-core.json |  6 ++++--
>  3 files changed, 27 insertions(+), 9 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>

(I’ve seen Markus’s concern, but I think management applications can
just see whether specifying sync={full,top} + bitmap works if they want
to use it.)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]